Re: [PATCH v5 2/3] drm/tidss: Update infrastructure to support K3 DSS cut-down versions

From: Tomi Valkeinen
Date: Mon May 05 2025 - 04:07:23 EST

Hi,

On 05/05/2025 10:49, Devarsh Thakkar wrote:
Hi Tomi

On 03/05/25 14:14, Tomi Valkeinen wrote:
On 02/05/2025 14:52, Devarsh Thakkar wrote:

Hi,


<snip>
Ok I see what you mean to say.....although functionally it is working
fine but from readability point of view it is confusing since both
functions use same argument name i.e hw_plane in two different contexts.
In that case, I would propose to use hw_id as arg name for all
dispc_k3_vid* functions, will that be okay ?

I'd prefer to have all the dispc functions take the same kind of index.


Why? Even all dispc functions are not named with same prefix.
1) dispc_vid* functions act on VID* base directly and here plane
indexing would be w.r.t which VID* base we are using e.g VID vs VIDL
2) dispc_k3_vid* functions act on common region bits which are related
to VID pipelines and plane indexing would signify vid base w.r.t common
register space i.e. COMMON_VID_IRQ0 vs COMMON_VID_IRQ1.

As they both act on different register base and refer it in different
contexts (VID* base vs COMMON_VID* base) and have also been named
differently anyway, I feel it is okay and legitimate to use hw_id for
dispc_k3_vid* functions (which would signify vid* indexing w.r.t common
region) and hw_plane for dispc_vid* functions (which would signify vid*
base w.r.t VID* regions mapped in device-tree).

I'm sorry, I don't understand your argument. Say, if there's code that first enables a plane and then wants to read the irqstatus for that plane, your argument is that it's better that the plane indices used when calling those functions are not the same? Because the called functions internally access the data in different ways?

Why does it matter if inside the functions the accessed bits are in different register blocks? It's about the same plane. Doesn't it make more sense to refer to the plane using the same index number?

Tomi