Re: [RFC PATCH] drm/panel: Make backlight attachment lazy

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] drm/panel: Make backlight attachment lazy
Date: Tue, 8 Dec 2020 16:02:16 -0600	[thread overview]
Message-ID: <X8/36HXL1IYPXA0J@builder.lan> (raw)
In-Reply-To: <X891/LvEJT1bLtjH@ulmo>

On Tue 08 Dec 06:47 CST 2020, Thierry Reding wrote:

> On Mon, Dec 07, 2020 at 10:44:46PM -0600, Bjorn Andersson wrote:
> > Some bridge chips, such as the TI SN65DSI86 DSI/eDP bridge, provides
> > means of generating a PWM signal for backlight control of the attached
> > panel. The provided PWM chip is typically controlled by the
> > pwm-backlight driver, which if tied to the panel will provide DPMS.
> > 
> > But with the current implementation the panel will refuse to probe
> > because the bridge driver has yet to probe and register the PWM chip,
> > and the bridge driver will refuse to probe because it's unable to find
> > the panel.
> 
> What you're describing is basically a circular dependency. Can't we get
> rid of that in some other way? Why exactly does the bridge driver refuse
> to probe if the panel can't be found?
> 
> In other words, I see how the bridge would /use/ the panel in that it
> forward a video stream to it. But how does the panel /use/ the bridge?
> 

Yes, this is indeed a circular dependency between the components.

The involved parts are:
* the bridge driver that implements the PWM chip probe defers on
  drm_of_find_panel_or_bridge() failing to find the panel.
* the pwm-backlight driver that consumes the PWM channel probe defer
  because the pwm_chip was not registered by the bridge.
* the panel that uses the backlight for DPMS purposes probe defer
  because drm_panel_of_backlight() fails to find the pwm-backlight.

I looked at means of postponing drm_of_find_panel_or_bridge() to
drm_bridge_funcs->attach(), but at that time "deferral" would be fatal.
I looked at registering the pwm_chip earlier, but that would depend on a
guarantee of the pwm-backlight and panel driver to probe concurrently.
And the current solution of not tying the backlight to the panel means
that when userspace decides to DPMS the display the backlight stays on.

The proposed solution (hack?) means that DPMS operations happening
before the pwm-backlight has probed will be missed, so it's not perfect.
It does however allow the backlight on my laptop to turn off, which is a
big improvement.

But I'm certainly welcome to suggestions.

Regards,
Bjorn