On Mon, Apr 28, 2014 at 9:08 AM, Ajay kumar <ajaynumb@xxxxxxxxx> wrote:
> Daniel,
>
> On Sun, Apr 27, 2014 at 6:25 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>> On Fri, Apr 25, 2014 at 8:17 AM, Ajay kumar <ajaynumb@xxxxxxxxx> wrote:
>>> We can call panel_enable/disable at the right point. Even the bridge chips
>>> expect the same. So, I am not ok with combining the bridge and panel and
>>> calling the fxn pointers from crtc_helpers.
>>> So, either we leave it the way it is in this patch (call drm_panel
>>> functions at right points) or
>>> we don't use drm_panel to control the panel sequence and instead use few DT
>>> properties and regulators, inside the bridge chip driver.
>>
>> That wasn't really suggested, but instead the idea was to provide a
>> default drm_bridge which wraps the drm_panel so that you could more
>> easily chain them up.
> What do you mean by "default" drm_bridge?
> I just want to clear few things. Let me explain what I have understood:
> What I understand is that Rob wants me to create a common
> structure which wraps up drm_panel and drm_bridge into a single structure:
> struct drm_panel_bridge {
> struct drm_bridge base;
> struct drm_panel *panel;
> struct drm_bridge *bridge; /* optional */ ==> Really not sure why
> this is needed!
> };
>
> static void drm_panel_bridge_enable(struct drm_bridge *bridge)
> {
> struct drm_panel_bridge *pb = to_panel_bridge(bridge);
> if (pb->bridge)
> pb->bridge->funcs->enable(pb->bridge);
> pb->panel->funcs->enable(pb->panel);
> }
>
> And now, the above function becomes a common function
> (also with an ordering issue btw panel and bridge).
> Where do we keep it? And, where do we call it from?
my proposal involves no change to crtc helpers. The later variation
(with composite_bridge so you could choose the order) would have a
sequence along the lines of:
crtc helpers:
+ bridge_funcs->enable(): -> composite_bridge_enable()
+ cbridge->b1->funcs->enable(): -> ptn3460_bridge_enable()
+ cbridge->b2->funcs->enable(): -> panel_bridge_enable();
+ pbridge->panel->funcs->enable(): -> foo_panel_enable()
or if the order needs to be swapped you can use ptn3460_bridge as b2
and panel bridge as b1.
BR,
-R
>> Also I'm not really happy that the bridge callbacks have been added to
>> the drm helpers (I'd prefer if driver callbacks would bear that
>> responsibility). But you can always create your own drm_bridge
>> integration.
> Do you mean, the bridge calls should be moved out of common drm_helper
> functions and called from platform specific drivers instead?
>
>> In any case your concern that drivers need to control
>> when certain callbacks are called is shared by everyone here afaics.
>> And I also don't see any issues with Rob's proposal in this regard.
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
> Please let me know if my understanding is correct and correct me if
> there is a misconception.
>
> Regards,
> Ajay Kumar
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
