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. [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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel