<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 23, 2016 at 10:01 PM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Aug 23, 2016 at 06:44:18PM +0200, Andrea Merello wrote:<br>
> On Tue, Aug 23, 2016 at 5:54 PM, Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>> wrote:<br>
><br>
> > On Tue, Aug 23, 2016 at 05:39:36PM +0200, Andrea Merello wrote:<br>
> > > On Tue, Aug 23, 2016 at 5:20 PM, Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>> wrote:<br>
> > ><br>
> > > > On Tue, Aug 23, 2016 at 04:08:04PM +0200, Andrea Merello wrote:<br>
> > > > > Introduce drm_simple_display_pipe_<wbr>attach_bridge() in order<br>
> > > > > to make it possible to use drm encoders with the simple display<br>
> > > > > pipes managed by simple_kms_helpers<br>
> > > > ><br>
> > > > > Suggested-by: Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>><br>
> > > > > Signed-off-by: Andrea Merello <<a href="mailto:andrea.merello@gmail.com">andrea.merello@gmail.com</a>><br>
> > > > > Cc: Noralf Trønnes <<a href="mailto:noralf@tronnes.org">noralf@tronnes.org</a>><br>
> > > > > Cc: Daniel Vetter <<a href="mailto:daniel@ffwll.ch">daniel@ffwll.ch</a>><br>
> > > > > Cc: David Airlie <<a href="mailto:airlied@linux.ie">airlied@linux.ie</a>><br>
> > > ><br>
> > > > Threading of your patch series is somehow broken, usually that should<br>
> > all<br>
> > > > work nicely if you've set up git send-email.<br>
> > > ><br>
> > > > One question: Should we ahve a drm_simple_display_pipe_<wbr>detach_bridge<br>
> > (for<br>
> > > > cleanup) too?<br>
> > > ><br>
> > ><br>
> > > Unsure if it worths. May be nice to have a balanced pair, but it would<br>
> > > probably end up in<br>
> > > a quite redundant one-line func, that only calls drm_bridge_detach with<br>
> > the<br>
> > > very same argument.<br>
> > ><br>
> > > ..But of course if you want I can add it in v2 series.<br>
> ><br>
> > Yes it's just going to be a one-line, but it'll do a typecast and so<br>
> > better encapsulate the internals of the simple pipe helper.<br>
><br>
><br>
> I'm unsure about what do you mean here. Why a typecast?<br>
><br>
> Wouldn't it be simply drm_simple_display_pipe_<wbr>detach_bridge(struct<br>
> drm_bridge *bridge) calling in turn drm_detach_bridge(struct drm_bridge<br>
> *bridge) with the very same argument?<br>
><br>
> Or if you want to stay behind the pipe, then it could be<br>
> drm_simple_display_pipe_<wbr>detach_bridge(struct drm_simple_display_pipe *pipe),<br>
> but I would say it just does something like<br>
> drm_bridge_detach(pipe-><wbr>encoder.bridge), so I don't really get your point<br>
> about the cast, sorry..<br>
<br>
</div></div>Yeah, the latter is what I mean. That way drivers don't have to dig around<br>
in the details of pipe. And it wouldn't be entirely just a wrapper, I<br>
think it'd be good to also clear pipe->encoder.bridge to NULL.<br>
<br></blockquote><div>Yes, since I'm assigning it in drm_simple_display_pipe_attach_bridge, it seems good to clear it in drm_simple_display_pipe_detach.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Or do you think this is entirely pointless? Sometimes I do go overboard<br>
with curating pretty little functions ;-)<br></blockquote><br>I guess it shouldn't hurt anyway :)<br><div> <br></div><div>Andrea<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">-Daniel<br>
--<br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="http://blog.ffwll.ch" rel="noreferrer" target="_blank">http://blog.ffwll.ch</a><br>
</div></div></blockquote></div><br></div></div>