Hi Daniel,
Le mar., août 10 2021 at 11:35:43 +0200, Daniel Vetter
<daniel@xxxxxxxx> a
écrit :
> On Fri, Aug 06, 2021 at 01:01:33PM +0200, Paul Cercueil wrote:
> > Hi Greg,
> >
> > Le ven., août 6 2021 at 12:17:55 +0200, Greg Kroah-Hartman
> > <gregkh@xxxxxxxxxxxxxxxxxxx> a écrit :
> > > On Thu, Aug 05, 2021 at 10:05:27PM +0200, Paul Cercueil
wrote:
> > > > Hi Greg,
> > > >
> > > > Le jeu., août 5 2021 at 21:35:34 +0200, Greg
Kroah-Hartman
> > > > <gregkh@xxxxxxxxxxxxxxxxxxx> a écrit :
> > > > > On Thu, Aug 05, 2021 at 09:21:09PM +0200, Paul Cercueil
> > wrote:
> > > > > > When the drivers of remote devices (e.g. HDMI chip)
are
> > > > disabled in
> > > > > > the
> > > > > > config, we want the ingenic-drm driver to be able to
probe
> > > > > > nonetheless
> > > > > > with the other devices (e.g. internal LCD panel)
that are
> > > > enabled.
> > > > > >
> > > > > > Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> > > > > > ---
> > > > > > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 12
> > ++++++++++++
> > > > > > 1 file changed, 12 insertions(+)
> > > > > >
> > > > > > diff --git
a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > > > > b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > > > > index d261f7a03b18..5e1fdbb0ba6b 100644
> > > > > > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > > > > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> > > > > > @@ -1058,6 +1058,18 @@ static int
ingenic_drm_bind(struct
> > > > device
> > > > > > *dev, bool has_components)
> > > > > > for (i = 0; ; i++) {
> > > > > > ret = drm_of_find_panel_or_bridge(dev->of_node,
0, i,
> > > > &panel,
> > > > > > &bridge);
> > > > > > if (ret) {
> > > > > > + /*
> > > > > > + * Workaround for the case where the drivers for
the
> > > > > > + * remote devices are not enabled. When that
happens,
> > > > > > + * drm_of_find_panel_or_bridge() returns
-EPROBE_DEFER
> > > > > > + * endlessly, which prevents the ingenic-drm
driver
> > from
> > > > > > + * working at all.
> > > > > > + */
> > > > > > + if (ret == -EPROBE_DEFER) {
> > > > > > + ret = driver_deferred_probe_check_state(dev);
> > > > > > + if (ret == -ENODEV || ret == -ETIMEDOUT)
> > > > > > + continue;
> > > > > > + }
> > > > >
> > > > > So you are mucking around with devices on other busses
> > within this
> > > > > driver? What could go wrong? :(
> > > >
> > > > I'm doing the same thing as everybody else. This is the
DRM
> > driver,
> > > > and
> > > > there is a driver for the external HDMI chip which gives
us a
> > DRM
> > > > bridge
> > > > that we can obtain from the device tree.
> > >
> > > But then why do you need to call this function that is there
for
> > a bus,
> > > not for a driver.
> >
> > The documentation disagrees with you :)
> >
> > And, if that has any weight, this solution was proposed by Rob.
> >
> > > > > Please use the existing driver core functionality for
this
> > type of
> > > > > thing, it is not unique, no need for this function to be
> > called.
> > > >
> > > > I'm not sure you understand what I'm doing here. This
driver
> > calls
> > > > drm_of_find_panel_or_bridge(), without guarantee that the
> > driver
> > > > for the
> > > > remote device (connected via DT graph) has been enabled
in the
> > > > kernel
> > > > config. In that case it will always return -EPROBE_DEFER
and
> > the
> > > > ingenic-drm
> > > > driver will never probe.
> > > >
> > > > This patch makes sure that the driver can probe if the
HDMI
> > driver
> > > > has been
> > > > disabled in the kernel config, nothing more.
> > >
> > > That should not be an issue as you do not care if the config
is
> > enabled,
> > > you just want to do something in the future if the driver
shows
> > up,
> > > right?
> >
> > Well, the DRM subsystem doesn't really seem to handle hotplug
of
> > hardware.
> > Right now all the drivers for the connected hardware need to
probe
> > before
> > the main DRM driver. So I need to know that a remote device
> > (connected via
> > DT graph) will never probe.
> >
> > Give me a of_graph_remote_device_driver_will_never_probe() and
I'll
> > use
> > that.
> >
> > > Much like the device link code, have you looked at that?
> >
> > I don't see how that would help in any way. The device link
code
> > would allow
> > me to set a dependency between the remote hardware (HDMI chip,
> > provider) and
> > the LCD controller (consumer), but I already have that
dependency
> > though the
> > DT graph. What I need is a way for the consumer to continue
probing
> > if the
> > provider is not going to probe.
>
> Is this actually a legit use-case?
>
> Like you have hw with a bunch of sub-devices linked, and you
decided to
> disable some of them, which makes the driver not load.
Yes. I'm facing that issue with a board that has a LCD panel and a
HDMI
controller (IT66121). I have a "flasher" program for all the
Ingenic boards,
that's basically just a Linux kernel + initramfs booted over USB
(device). I
can't realistically enable every single driver for all the hardware
that's
on these boards while still having a tiny footprint. And I
shouldn't have to
care about it either.
