Re: Armada DRM: bridge with componentized devices

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Jan 07, 2019 at 12:26:58PM +0100, Andrzej Hajda wrote:
> On 07.01.2019 11:45, Daniel Vetter wrote:
> > On Thu, Jan 03, 2019 at 01:11:47PM +0000, Russell King - ARM Linux wrote:
> >> On Thu, Jan 03, 2019 at 10:47:27AM +0100, Lubomir Rintel wrote:
> >>> Hello,
> >>>
> >>> lately I've been trying to make the Himax HX8837 chip that drives the OLPC
> >>> LCD display work with Armada DRM driver. I've been advised to create a
> >>> bridge driver and not an encoder driver since the silicon is separate from
> >>> the LCDC.
> >>>
> >>> The Armada DRM driver (and, I think, the i.MX one) creates the drm_device
> >>> once the component infrastructure sees the necessary sub-devices appear.
> >>> The sub-devices being the LCDCs and the encoders (not bridges) that it
> >>> expects to be created externally.
> >>>
> >>> Currently, it seems, the only driver that can actually work with this (that
> >>> is -- creates a drm_encoder for a drm_device when the component is bound)
> >>> is the tda998x. All other similar drivers create a drm_bridge instead and
> >>> not use the component infrastructure at all. (In fact, tilcdc driver
> >>> contains a  hack to handle tda998x specially.)
> >>>
> >>> I'm wondering how to reconcile the two?
> >>>
> >>> * The tda998x driver has recently been modified to create a bridge on probe
> >>>   and eventually encoder on component bind. Is this an okay thing to do in
> >>>   a new driver? (this probably means the tilcdc hack can be removed...)
> >>>
> >>> * If a non-componentized bridge were to be used (along with a dummy 
> >>>   encoder), at what point would it make sense to look for the bridge? 
> >>>   Would it be a good idea to defer the probe of crtc until a bridge can be 
> >>>   looked up and the attach it on component bind?  What if the bridge goes 
> >>>   away (a module is unloaded, etc.) in between?
> >>>
> >>> I'd be thankful for opintions and advice before I move ahead with this.
> >> This is the long-standing problem with the conflict between bridge
> >> support and component support, and I'm not sure that there is really
> >> any answer to it.
> >>
> >> I've gone into the details of the two several times on the list,
> >> particularly about the short-comings of the bridge approach, but it
> >> seems no one cares to fix those short-comings.
> >>
> >> You are re-identifying some of the issues that I've already pointed
> >> out - such as what happens to DRM drives when the bridge driver is
> >> unbound (it's really not about modules being unloaded, and the problem
> >> can't be solved by taking a module reference count - all that the
> >> module reference count does is ensure that the module doesn't go
> >> away unexpected, there is no way to ensure that the device isn't
> >> unbound.)
> >>
> >> The issue of unbinding is precisely the issue which the component
> >> support was created to solve - but everyone seems to prefer the buggy
> >> bridge approach, and no one seems willing to do anything about the
> >> bugs or even acknowledge that it's a problem.  It's strange - if one
> >> identifies bugs that result in kernel oops in other kernel subsystems,
> >> one is generally taken seriously and the problem is solved.
> > Unbinding is really not the most important feature, especially for SoC. If
> > you feel different, working together with others, getting some agreement,
> > getting the patches reviewed and finding someone to get them merged is
> > very much appreciated. But just complaining won't move this forward.
> >
> >> The issue about the encoders is something that I've tried to discuss,
> >> and I've pointed out that moving it into the DRM driver adds additional
> >> complexity there, and I'd hoped that my patch set I posted would've
> >> generated discussion, but alas not.
> >>
> >> What I'm not prepared to do is to introduce _known_ bugs into any
> >> driver that I maintain.
> > I thought last time around the idea was to use device links (and teach
> > drm_bridge about them), so that the unloading works correctly.
> 
> 
> With current device_links implementation registering links in probe of
> the consumer is just incorrect - it can happen that neither consumer
> neither provider is fully probed and creating device links in such state
> is wrong according to docs, and my experiments. See [1] for discussion
> and [2] for docs.

We could set up the device link only at drm_dev_register time. At that point
we know that driver loading has fully succeeded. We do have a list of
bridges at hand, so that's not going to be an issue.

The only case where this breaks is if a driver is still using the
deprecated ->load callback. But that ->load callback doesn't mix well with
EDEFER_PROBE/component framework anyway, so I think not going to be a
problem hopefully?
-Daniel
> 
> 
> [1]: https://patchwork.freedesktop.org/patch/218878/
> 
> [2]:
> https://www.kernel.org/doc/html/latest/driver-api/device_link.html#usage
> 
> 
> Regards
> 
> Andrzej
> 
> 
> >
> > Wrt tda988x: I think it really shouldn't create a drm_encoder, nor
> > register as a component. Fixing that is probably a bit more work.
> > -Daniel
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel