Re: [PATCH v3 0/7] drm/i2c: adv7511: ADV7533 support

[Xinliang Liu <xinliang.liu@linaro.org>]

On 18 April 2016 at 17:48, Archit Taneja <architt@codeaurora.org> wrote:
> Hi,
>
>
> On 04/17/2016 05:01 PM, Xinliang Liu wrote:
>>
>> Hi Archit,
>> Thank you for the patches.
>>
>> On 9 March 2016 at 18:57, Archit Taneja <architt@codeaurora.org> wrote:
>>>
>>> ADV7533 is a DSI to HDMI encoder chip. It's like ADV7511, but with an
>>> additional DSI RX block that takes in DSI video mode output.
>>>
>>> Trying to get this driver merged has had some challenges:
>>>
>>> - ADV7533 has an I2C control bus, but acts as a DSI peripheral too.
>>>    After discussions, it was concluded that we'd want to provide an
>>>    API to create MIPI DSI devices, rather than expose two different
>>>    interfaces on DT. The first version [1] tried the former approach
>>>    the second version [2] showed how the driver would look like if
>>>    exposed 2 DT nodes. This lateset patchset relies on the MIPI DSI
>>>    device creation API provided by [3], this has been accepted and
>>>    should be merged for 4.6.
>>>
>>> - The driver was designed as an I2C slave encoder. When ADV7533
>>>    patches were posted [1], it was modelled as a bridge, but ADV7511
>>>    and others were still left as I2C slave encoders. This wasn't
>>>    accepted. After discussions, it was decided that ADV7511 too would
>>>    be converted into a bridge driver, and all the users of ADV7511
>>>    should assume it is a bridge. This bridge conversion was done in
>>>    [4]. There is still some debate over whether the bridge driver be
>>>    involved in the connector creation, or the KMS driver that has
>>>    the whole view of the display pipeline. This discussion shouldn't
>>>    affect this patch set, though.
>>
>>
>> I also agree with Laurent Pinchart that bridge driver shoudn't get
>> involved in in the connector creation.
>> It is better for KMS driver that has the whole view of the display
>> pipeline.
>
>
> Yes, that's the eventual plan. We were thinking of creating an
> additional drm bridge api (drm_bridge_create_connector) that
> helps the KMS driver create a connector for the bridge.
>
> Since there are  certain connector related properties that the KMS
> driver may not be aware of, we were thinking of creating another
> drm_bridge op which fills up connector properties. Some properties
> (like whether we want POLLED HPD or not) are more platform specific,
> and would be parsed via the connector's DT node (that is, if DT is
> supported on that platform)
>
> For now, this series creates the connector in the bridge's
> attach op, but relies on the KMS driver to call
> drm_connector_register. The other stuff I mentioned above can come
> later.

This is great, if we have the plan.

>
>>
>> In this case, I mean creating bridge driver to support ADV7533. I
>> think this is something look like panel driver. Maybe we need to add
>> some callback to the bridge to avoid creating connector in bridge
>> driver.
>> We can refer to panel entity to see what callbacks to be added. One
>> callback I can see is the get_modes callback might be need.
>>
>>>
>>> This patch set enables ADV7533 support with the above two issues
>>> now resolved. It also incorporates ADV7533 specific features and fixes
>>> that we've discovered since the first version of this patch was posted.
>>>
>>> Tested on ADV7533 chips on DB410c. It should work on the Hikey board too.
>>
>>
>> I have tested the this tracking branch :
>>
>> https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/shortlog/refs/heads/tracking-qcomlt-adv7511
>> it works for HiKey.
>> But it seems this patch set is a little different from the above
>> branch. I think I need to tested this patch set.
>
>
> Yes, it would be great if you could test the posted patch set and
> provided a Tested-by. The patches "drm/i2c: adv7511: Init regulators"
> and above need to still be applied from this branch. I'll post these
> out in another patch set later.

Sure, I will test them all.

Thanks,
-xinliang

>
> Thanks,
> Archit
>
>
>>
>> Thanks,
>> -xinliang
>>
>>> I'd appreaciate if someone could test it on a ADV7511 platform since I
>>> don't have one.
>>>
>>> [4]
>>> https://lists.freedesktop.org/archives/dri-devel/2016-January/098287.html
>>>
>>> [3]
>>> https://lkml.org/lkml/2016/2/12/67
>>>
>>> [2]
>>>
>>> https://lists.freedesktop.org/archives/dri-devel/2015-September/089884.html
>>>
>>> [1]:
>>> https://lists.freedesktop.org/archives/dri-devel/2015-July/087088.html
>>>
>>> Archit Taneja (7):
>>>    drm/i2c: adv7511: Convert to drm_bridge
>>>    drm/i2c: adv7511: Fix mutex deadlock when interrupts are disabled
>>>    drm/i2c: adv7511: Initial support for ADV7533
>>>    drm/i2c: adv7511: Create a MIPI DSI device
>>>    drm/i2c: adv7511: Use internal timing generator
>>>    drm/i2c: adv7511: Change number of DSI lanes dynamically
>>>    dt-bindings: drm/bridge: Update bindings for ADV7533
>>>
>>>   .../bindings/display/bridge/adi,adv7511.txt        |  25 +-
>>>   drivers/gpu/drm/i2c/adv7511.c                      | 539
>>> +++++++++++++++++----
>>>   2 files changed, 476 insertions(+), 88 deletions(-)
>>>
>>> --
>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>>> Forum,
>>> hosted by The Linux Foundation
>>>
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel