Hi Daniel, On Tuesday 29 Nov 2016 21:25:27 Daniel Vetter wrote: > On Tue, Nov 29, 2016 at 07:49:22PM +0200, Laurent Pinchart wrote: > > On Tuesday 29 Nov 2016 11:27:20 Daniel Vetter wrote: > >> On Tue, Nov 29, 2016 at 11:58:44AM +0200, Laurent Pinchart wrote: > >>> On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote: > >>>> On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote: > >>>>> The drm_bridge object models on- or off-chip hardware encoders and > >>>>> provide an abstract control API to display drivers. In order to help > >>>>> display drivers creating the right kind of drm_encoder object, > >>>>> expose the type of the hardware encoder associated with each bridge. > >>>>> > >>>>> Signed-off-by: Laurent Pinchart > >>>>> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > >>>> > >>>> DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one > >>>> cares one iota about the encoder type. > >>> > >>> It's exposed to userspace though, are you 100% sure we won't break > >>> anything ? > >> > >> We've added DP, DSI, DPMST and DPI encoder types thus far, no one > >> screamed. > > > > In that case why don't we go one step further and remove the encoder type > > completely ? We can't remove the field from the API, but we can hardcode > > it to a single value. > > > > There are however drivers that rely on the encoder type (radeon, nouveau, > > sti, amdgpu, msm and rcar-du, but I'll fix the last one) so we'd need to > > address that first. If we don't want to remove the encoder_type field > > from in-kernel structures and let drivers use it, then I don't think > > DRM_MODE_ENCODER_BRIDGE would be a good option, we should report the real > > type instead. > > If you strongly believe that I will not stop you. This was just a > suggestion to get all your stuff landed with minimal amounts of effort and > across-the-subsystem cleanup needed. I'd do it that ;-) > > And if you don't like DRM_MODE_ENCODER_BRIDGE you could also pick > DRM_MODE_ENCODER_NONE, which is what most seem to do today. In the end it > doesn't matter no matter which option you pick. The only difference is in > the amount of effort you need to spend to get it merged ... I've tried hardcoding the encoder type to DRM_MODE_ENCODER_NONE and basic tests still pass (I haven't tried more complex userspace stacks such as X or Weston though). I'll thus drop the addition of encoder_type to the bridge for now. As a result we should start deprecating the drm_encoder::encoder_type field (unless a compelling reason is found of course, in which case I'd have to revive drm_bridge::encoder_type). -- Regards, Laurent Pinchart
