Re: [PATCH v3 1/2] drm/bridge: ite-it66121: Set TX mode in the .atomic_enable callback

[Javier Martinez Canillas]

Maxime Ripard <mripard@xxxxxxxxxx> writes:

> On Tue, May 12, 2026 at 08:54:13PM +0200, Javier Martinez Canillas wrote:

[...]

>> >
>> > Having some part of it in mode_set and some part in enable is still kind
>> > of weird. The best there would be to put everything in enable (and
>> > pre_enable), and drop mode_set entirely.
>> >
>> 
>> I agree that this will be the correct approach. I wonder though if could be
>> acceptable to land the changes in this series as a minimal fix for the DVI
>> mode issue, and then do further refactoring as a follow-up.
>
> The thing is that you're now making the driver and its interaction with
> the hardware more complex. Like, what would happen if mode_set is now
> called after atomic_enable? How does the hardware behavies if you modify
> the SII902X_SYS_CTRL_DATA register only after it's been enabled?
>

That's a good point.

> Moving everything to atomic_enable keeps things simple and keep the
> hardware access ordering the same.
>

Yes, I agree.

> And if you don't plan on sending that to fixes, the fact that the patch
> is small is kind of irrelevant anyway.
>

That's true too. I'll then prepare a v4 doing it properly then even if may
be a bigger refactoring.

Thanks again for the feedback and comments!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat