Maxime Ripard <[email protected]> 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
