On 11.12.2017 17:27, Thierry Reding wrote: > On Mon, Dec 11, 2017 at 04:53:56PM +0300, Dmitry Osipenko wrote: >> On 11.12.2017 13:13, Thierry Reding wrote: >>> On Mon, Dec 11, 2017 at 02:19:44AM +0300, Dmitry Osipenko wrote: >>>> Add manual HW power management to drivers probe/remove in order to >>>> not fail in a case of runtime power management being disabled in kernel >>>> config. >>>> >>>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> >>>> --- >>>> drivers/gpu/drm/tegra/dc.c | 164 +++++++++++++++++++++++++++---------------- >>>> drivers/gpu/drm/tegra/dsi.c | 138 +++++++++++++++++++++--------------- >>>> drivers/gpu/drm/tegra/hdmi.c | 90 ++++++++++++++++-------- >>>> drivers/gpu/drm/tegra/sor.c | 103 +++++++++++++++++---------- >>>> 4 files changed, 310 insertions(+), 185 deletions(-) >>> >>> I think that's the wrong way around. We unconditionally select PM on >>> 64-bit ARM already, and I think we should do the same on 32-bit ARM. >>> There's really no excuse not to enable runtime PM these days. >> >> What is the rational behind enabling PM unconditionally? It is actually a very >> useful debug feature when there is something wrong with the PM. It looks like >> Tegra DRM driver is the only driver on Tegra that doesn't work properly with PM >> being disabled. Please, let's just fix it. > > What's useful about disabling PM? The problem with allowing !PM is that > it adds one more combination that needs to be build- and runtime tested. As I already stated, disabling PM is very useful for debugging when system hangs unexpectedly. I found it very helpful several times. > It also means we need to have extra code in the drivers to enable the > device when runtime PM is disabled. This is all a lot of extra effort > for little to no benefit. Nearly every driver has a dozen lines of extra code to handle disabled PM. I don't see why Tegra DRM should be an excuse. Also notice that I added some more error messages. Of course it would be nice if you could test disabled PM config option from time to time. I don't think it is very important config option for a regular user, but very useful for developing and for debugging when things break. So it shouldn't an extra testing burden for you. > Also, runtime PM is an integral part of mode setting in Tegra DRM, so I > would be suprised if things still work reliably after this patch. RGB, > DSI and HDMI might work properly, but the SOR is unlikely to work after > this because the only way to get it to reliably set a mode is by using > the full runtime suspend/resume cycle. I wasn't aware of the SOR's hazard, thanks for pointing at it. Probably performing power cycle on encoders enabling would be enough, wouldn't it? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html