lkml.org 
[lkml]   [2023]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v7 3/6] drm/tidss: Add support for AM625 DSS
From
On 05/02/2023 16:31, Aradhya Bhatia wrote:
>
>
> On 03-Feb-23 21:03, Tomi Valkeinen wrote:
>> On 25/01/2023 13:35, Aradhya Bhatia wrote:
>>> Add support for the DSS controller on TI's new AM625 SoC in the tidss
>>> driver.
>>>
>>> The first video port (VP0) in am625-dss can output OLDI signals through
>>> 2 OLDI TXes. A 3rd output port has been added with "DISPC_PORT_OLDI" bus
>>> type.
>>
>> Not a big thing here as you add support for a new SoC, but the ordering
>> of the patches is not optimal. Here you add the AM625 DSS support, but
>> then you continue actually adding the DSS support (well, mainly OLDI) in
>> the following patches.
>>
>> I think patch 6 could be before this patch. Parts of patch 4 could also
>> be before this patch. The AM65X renames from patch 5 could be before
>> this patch.
>
> I can move whole of Patch 6 and even of Patch 4 before this one. I have
> mentioned 'AM625-DSS' in a couple comments which I can make generic,
> and the rest everything is SoC-agnostic.
>
> I haven't tried this, but my concern is if we break patch 5 into 2
> separate patches,
>
> i. AM65X rename plus SoC based switch case, and
> ii. Addition of AM625 SoC case
>
> then I might have to overwrite some changes implemented during (i) in
> (ii). I don't suppose that would be okay, would it?

I'm not sure I follow here. Wouldn't (i) be a valid patch in its own?
Nothing wrong in expanding that later (even if you end up changing a lot
of it).

That said, I don't think this is a very important topic. There are only
a few commits in the history that might be problematic. A simple fix
would be to add all the features first, and only last add the compatible
string for am625.

Or do all the changes for am625 in a single patch, and try to implement
all the generic restructuring work before that.

Here we do have to change the vp-to-output mapping management, so maybe
the second option won't be simple enough, and it's better to do the
am625 changes in pieces, as in the first option.

So, it's really up to you. Just wanted to raise this possible issue so
that you are aware of it and can do any easy fixes (if there are such).

> Also, is it important to keep the compatible-addition patches of
> DT-binding and driver next to each other in the series? Or should
> the DT-binding patches should be the first ones? Just curious! =)

I believe the convention is to have the DT-binding changes before you
add the compatible string to the driver (if I recall right checkpatch or
some other checking tool complains if you add a driver for a compatible
that doesn't have a DT binding). Generic restructurings could be before
the DT patch, of course, but usually I like to keep the DT binding
changes at the very beginning of the series.

Tomi

\
 
 \ /
  Last update: 2023-03-27 00:10    [W:0.068 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site