Re: [PATCH 2/2] drm: simple_kms_helper: add support for bridges

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 23, 2016 at 06:44:18PM +0200, Andrea Merello wrote:
> On Tue, Aug 23, 2016 at 5:54 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> 
> > On Tue, Aug 23, 2016 at 05:39:36PM +0200, Andrea Merello wrote:
> > > On Tue, Aug 23, 2016 at 5:20 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> > >
> > > > On Tue, Aug 23, 2016 at 04:08:04PM +0200, Andrea Merello wrote:
> > > > > Introduce drm_simple_display_pipe_attach_bridge() in order
> > > > > to make it possible to use drm encoders with the simple display
> > > > > pipes managed by simple_kms_helpers
> > > > >
> > > > > Suggested-by: Daniel Vetter <daniel@xxxxxxxx>
> > > > > Signed-off-by: Andrea Merello <andrea.merello@xxxxxxxxx>
> > > > > Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > > > > Cc: Daniel Vetter <daniel@xxxxxxxx>
> > > > > Cc: David Airlie <airlied@xxxxxxxx>
> > > >
> > > > Threading of your patch series is somehow broken, usually that should
> > all
> > > > work nicely if you've set up git send-email.
> > > >
> > > > One question: Should we ahve a drm_simple_display_pipe_detach_bridge
> > (for
> > > > cleanup) too?
> > > >
> > >
> > > Unsure if it worths. May be nice to have a balanced pair, but it would
> > > probably end up in
> > > a quite redundant one-line func, that only calls drm_bridge_detach with
> > the
> > > very same argument.
> > >
> > > ..But of course if you want I can add it in v2 series.
> >
> > Yes it's just going to be a one-line, but it'll do a typecast and so
> > better encapsulate the internals of the simple pipe helper.
> 
> 
> I'm unsure about what do you mean here. Why a typecast?
> 
> Wouldn't it be simply drm_simple_display_pipe_detach_bridge(struct
> drm_bridge *bridge) calling in turn drm_detach_bridge(struct drm_bridge
> *bridge) with the very same argument?
> 
> Or if you want to stay behind the pipe, then it could be
> drm_simple_display_pipe_detach_bridge(struct drm_simple_display_pipe *pipe),
> but I would say it just does something like
> drm_bridge_detach(pipe->encoder.bridge), so I don't really get your point
> about the cast, sorry..

Yeah, the latter is what I mean. That way drivers don't have to dig around
in the details of pipe. And it wouldn't be entirely just a wrapper, I
think it'd be good to also clear pipe->encoder.bridge to NULL.

Or do you think this is entirely pointless? Sometimes I do go overboard
with curating pretty little functions ;-)
-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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux