Hi, On Sun, May 5, 2024 at 11:52 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > > On Fri, May 3, 2024 at 11:36 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote: > > > As talked about in commit d2aacaf07395 ("drm/panel: Check for already > > prepared/enabled in drm_panel"), we want to remove needless code from > > panel drivers that was storing and double-checking the > > prepared/enabled state. Even if someone was relying on the > > double-check before, that double-check is now in the core and not > > needed in individual drivers. > > > > This series attempts to do just that. While the original grep, AKA: > > git grep 'if.*>prepared' -- drivers/gpu/drm/panel > > git grep 'if.*>enabled' -- drivers/gpu/drm/panel > > ...still produces a few hits after my series, they are _mostly_ all > > gone. The ones that are left are less trivial to fix. > > > > One of the main reasons that many panels probably needed to store and > > double-check their prepared/enabled appears to have been to handle > > shutdown and/or remove. Panels drivers often wanted to force the power > > off for panels in these cases and this was a good reason for the > > double-check. > > > > In response to my V1 series [1] we had much discussion of what to > > do. The conclusion was that as long as DRM modeset drivers properly > > called drm_atomic_helper_shutdown() that we should be able to remove > > the explicit shutdown/remove handling in the panel drivers. Most of > > the patches to improve DRM modeset drivers [2] [3] [4] have now > > landed. > > > > In contrast to my V1 series, I broke the V2 series up a lot > > more. Since a few of the panel drivers in V1 already landed, we had > > fewer total drivers and so we could devote a patch to each panel. > > Also, since we were now relying on DRM modeset drivers I felt like we > > should split the patches for each panel into two: one that's > > definitely safe and one that could be reverted if we found a > > problematic DRM modeset driver that we couldn't fix. > > > > Sorry for the large number of patches. I've set things to mostly just > > CC people on the cover letter and the patches that are relevant to > > them. I've tried to CC people on the whole series that have shown > > interest in this TODO item. > > > > As patches in this series are reviewed and/or tested they could be > > landed. There's really no ordering requirement for the series unless > > patches touch the same driver. > > > > NOTE: this touches _a lot_ of drivers, is repetitive, and is not > > really possible to generate automatically. That means it's entirely > > possible that my eyes glazed over and I did something wrong. Please > > double-check me and don't assume that I got everything perfect, though > > I did my best. I have at least confirmed that "allmodconfig" for arm64 > > doesn't fall on its face with this series. I haven't done a ton of > > other testing. > > > > [1] https://lore.kernel.org/r/20230804140605.RFC.4.I930069a32baab6faf46d6b234f89613b5cec0f14@changeid > > [2] https://lore.kernel.org/r/20230901234015.566018-1-dianders@xxxxxxxxxxxx > > [3] https://lore.kernel.org/r/20230901234202.566951-1-dianders@xxxxxxxxxxxx > > [4] https://lore.kernel.org/r/20230921192749.1542462-1-dianders@xxxxxxxxxxxx > > This is the right thing to do, thanks for looking into this! > > As for the behaviour of .remove() I doubt whether in many cases > the original driver authors have even tested this themselves. Yeah, I'd tend to agree. > I would say we should just apply the series as soon as it's non-RFC It's not actually RFC now, but "RFT" (request for testing). I don't _think_ there's any need to send a version without the RFT tag before landing unless someone really feels strongly about it. > after the next merge window With drm-misc there's not really any specific reason to wait for the merge window to open/close as we can land in drm-misc-next at any time regardless of the merge window. drm-misc-next will simply stop feeding linuxnext for a while. That all being said, I'm happy to delay landing this until after the next -rc1 comes out if people would prefer that. If I don't hear anything, I guess I'll just wait until -rc1 before landing any of these. > and see what happens. I doubt it > will cause much trouble. I can land the whole series if that's what everyone agrees on. As I mentioned above, I'm at least slightly worried that I did something stupid _somewhere_ in this series since no automation was possible and with repetitive tasks like this it's super easy to flub something up. It's _probably_ fine, but I guess I still have the worry in the back of my mind. If folks think I should just apply the whole series then I'm happy to do that. If folks think I should just land parts of the series as they are reviewed/tested I can do that as well. Let me know. If I don't hear anything I'd tend to just land patches that are reviewed/tested. Then after a month or so (hopefully) I'd send out a v2 with anything left. > The series: > Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Thanks! -Doug