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