[Freedreno] [PATCH v4 0/3] drm: simplify support for transparent DRM bridges

[Laurent Pinchart]

Hi Dmitry,

On Mon, Sep 04, 2023 at 12:02:18AM +0300, Dmitry Baryshkov wrote:
> On Tue, 22 Aug 2023 at 17:17, Laurent Pinchart wrote:
> > On Thu, Aug 17, 2023 at 05:55:13PM +0300, Dmitry Baryshkov wrote:
> > > Supporting DP/USB-C can result in a chain of several transparent
> > > bridges (PHY, redrivers, mux, etc). This results in drivers having
> > > similar boilerplate code for such bridges.
> >
> > What do you mean by transparent bridge here ? Bridges are a DRM concept,
> > and as far as I can tell, a PHY isn't a bridge. Why does it need to be
> > handled as one, especially if it's completely transparent ?
> >
> > > Next, these drivers are susceptible to -EPROBE_DEFER loops: the next
> > > bridge can either be probed from the bridge->attach callback, when it is
> > > too late to return -EPROBE_DEFER, or from the probe() callback, when the
> > > next bridge might not yet be available, because it depends on the
> > > resources provided by the probing device.
> >
> > Can't device links help avoiding defer probing in those cases ?
> 
> It looks like both Neil and I missed this question.

And I missed this reply before replying to Neil and pointing to device
links again :-)

> Two items wrt devlinks. First, I view them as a helper. So if one
> disables the devlinks enforcement, he'd still get a deferral loop.

That may be true, but I don't think that's a compelling argument. If one
disables components meant to avoid probe deferral, they should expect
probe deferral :-)

> Second, in this case we can not enforce devlinks (or return
> -EPROBE_DEFER from the probe() function) because the next bridge is
> not yet available when the main driver probes. Unfortunately bridges
> are allocated in the opposite order. So, using AUX devices helps us to
> break it. Because first typec mux/retimer/switch/etc devices probe (in
> the direction from the typec source to the typec port). Then DRM
> bridge devices are probed starting from the end of the chain
> (connector) to the DP source (root DP bridge/controller).

I'm not too familiar with the drivers involved in the typec chain. Do
you mean that the sink driver needs to get hold of the source device at
probe time, deferring probe if the source is not available ? Does the
driver handler the USB connector need to do the same ? I'm looking at
the qcom_pmic_typec driver and I don't immediately see where it would
defer probing if its source isn't available, but I may well be missing
something.

> > > Last, but not least, this results in the the internal knowledge of DRM
> > > subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC.
> >
> > Why so ? The PHY subsystem should provide a PHY, without considering
> > what subsystem it will be used by. This patch series seems to me to
> > actually create this DRM dependency in other subsystems, which I don't
> > think is a very good idea. Resources should be registered in their own
> > subsystem with the appropriate API, not in a way that is tied to a
> > particular consumer.
> >
> > > To solve all these issues, define a separate DRM helper, which creates
> > > separate aux device just for the bridge. During probe such aux device
> > > doesn't result in the EPROBE_DEFER loops. Instead it allows the device
> > > drivers to probe properly, according to the actual resource
> > > dependencies. The bridge auxdevs are then probed when the next bridge
> > > becomes available, sparing drivers from drm_bridge_attach() returning
> > > -EPROBE_DEFER.
> >
> > I'm not thrilled :-( Let's discuss the questions above first.
> 
> Laurent, please excuse me for the ping. Any further response from your side?
> I'd like to send the next iteration of this patchset.
> 
> > > Proposed merge strategy: immutable branch with the drm commit, which is
> > > then merged into PHY and USB subsystems together with the corresponding
> > > patch.

-- 
Regards,

Laurent Pinchart