On Tue, May 6, 2014 at 1:03 PM, Ajay kumar <ajaynumb@xxxxxxxxx> wrote:
> Sean,
>
>
> On Tue, May 6, 2014 at 9:24 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
>> On Mon, May 5, 2014 at 12:52 PM, Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx> wrote:
>>> modify the driver to call the bridge->funcs of the next bridge
>>> in the chain.
>>> We assume that the bridge sitting next to ptn3460 is a bridge-panel.
>>>
>>> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/bridge/ptn3460.c | 21 +++++++++++++++++----
>>> 1 file changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
>>> index b171901..969869a 100644
>>> --- a/drivers/gpu/drm/bridge/ptn3460.c
>>> +++ b/drivers/gpu/drm/bridge/ptn3460.c
>>> @@ -126,6 +126,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
>>> gpio_set_value(ptn_bridge->gpio_rst_n, 1);
>>> }
>>>
>>> + if (bridge->next_bridge)
>>> + bridge->next_bridge->funcs->pre_enable(bridge->next_bridge);
>>> /*
>>> * There's a bug in the PTN chip where it falsely asserts hotplug before
>>> * it is fully functional. We're forced to wait for the maximum start up
>>> @@ -142,6 +144,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
>>>
>>> static void ptn3460_enable(struct drm_bridge *bridge)
>>> {
>>> + if (bridge->next_bridge)
>>> + bridge->next_bridge->funcs->enable(bridge->next_bridge);
>>> }
>>>
>>> static void ptn3460_disable(struct drm_bridge *bridge)
>>> @@ -153,6 +157,11 @@ static void ptn3460_disable(struct drm_bridge *bridge)
>>>
>>> ptn_bridge->enabled = false;
>>>
>>> + if (bridge->next_bridge) {
>>> + bridge->next_bridge->funcs->disable(bridge->next_bridge);
>>> + bridge->next_bridge->funcs->post_disable(bridge->next_bridge);
>>
>> Shouldn't post_disable be called from ptn_3460_post_disable, instead of here?
> Umm, right. But no point in delaying ->post_disable of the panel(which
> switches off power to LCD) since
> backlight would be already down in ->disable call itself. So, I
> thought of calling post_disable here itself.
>
>>> + }
>>> +
>>> if (gpio_is_valid(ptn_bridge->gpio_rst_n))
>>> gpio_set_value(ptn_bridge->gpio_rst_n, 1);
>>>
>>> @@ -197,7 +206,10 @@ int ptn3460_get_modes(struct drm_connector *connector)
>>> return drm_add_edid_modes(connector, ptn_bridge->edid);
>>>
>>> power_off = !ptn_bridge->enabled;
>>> - ptn3460_pre_enable(ptn_bridge->bridge);
>>> + if (power_off) {
>>> + ptn3460_pre_enable(ptn_bridge->bridge);
>>> + ptn3460_enable(ptn_bridge->bridge);
>>
>> In this case, I don't think we need to power on the entire bridge
>> chain since we're just reading the edid from the bridge chip itself.
>> So I think I'd prefer to break out the power_on/power_off code from
>> the bridge chain such that we can just power up the bridge chip, check
>> the edid and then turn it off.
> Those extra calls were added to make sure that regulator_enable/disable would
> be in sync for the panel. Check the other patch [2/3].
Sure, but we don't need the panel to read the edid. This bridge in
particular provides the edid itself, so in this case, we should leave
the panel off.
Sean
> Another way of doing this is to just check if the bridge is off and
> switch it on by
> explicitly setting up the gpios here, instead of just calling
> ptn3460_pre_enable.
>
> Ajay
>> In other bridges, where we're actually reading the edid from a
>> downstream source, this decision might be different.
>>
>>
>>
>>> + }
>>>
>>> edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
>>> if (!edid) {
>>> @@ -219,8 +231,10 @@ int ptn3460_get_modes(struct drm_connector *connector)
>>> num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
>>>
>>> out:
>>> - if (power_off)
>>> + if (power_off) {
>>> ptn3460_disable(ptn_bridge->bridge);
>>> + ptn3460_post_disable(ptn_bridge->bridge);
>>> + }
>>>
>>> return num_modes;
>>> }
>>> @@ -318,14 +332,13 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
>>> goto err;
>>> }
>>>
>>> - ret = drm_bridge_init(dev, bridge, &ptn3460_bridge_funcs);
>>> + ret = drm_bridge_init(dev, bridge, encoder, &ptn3460_bridge_funcs);
>>> if (ret) {
>>> DRM_ERROR("Failed to initialize bridge with drm\n");
>>> goto err;
>>> }
>>>
>>> bridge->driver_private = ptn_bridge;
>>> - encoder->bridge = bridge;
>>>
>>> ret = drm_connector_init(dev, &ptn_bridge->connector,
>>> &ptn3460_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
>>> --
>>> 1.8.1.2
>>>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
