Re: [PATCH 3/5] drm/i2c: adv7511: Refactor encoder slave functions

[Archit Taneja <architt@codeaurora.org>]

On 12/3/2015 9:25 PM, Rob Clark wrote:
> On Thu, Dec 3, 2015 at 10:28 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>> On Thursday 03 December 2015 10:02:02 Rob Clark wrote:
>>> On Mon, Jul 27, 2015 at 4:59 AM, Laurent Pinchart wrote:
>>>> On Monday 27 July 2015 11:46:57 Archit Taneja wrote:
>>>>> ADV7511 is represented as an i2c drm slave encoder device. ADV7533, on
>>>>> the other hand, is going be a normal i2c client device creating bridge
>>>>> and connector entities.
>>>>
>>>> Please, no. It's really time to stop piling hacks and fix the problem
>>>> properly. There's no reason to have separate APIs for I2C slave encoders
>>>> and DRM bridges. Those two APIs need to be merged, and then you'll find
>>>> it much easier to implement ADV7533 support.
>>>
>>> btw, what is the status here?  I'd kind of lost track of the
>>> discussion, but I'm getting impatient that it is somehow taking
>>> ridiculously long to get adv7533 support merged.  (It's good thing the
>>> x86/desktop folks don't bikeshed so much..  I'd hate to wait a year
>>> for my new laptop to be supported..)
>>
>> I'd hardly call the overall architecture on which drivers rely a bikeshed (and
>> maybe if x86/desktop folks cared more about embedded there would be more
>> willingness to make the framework evolve in an embedded-friendly way).
>
> I don't know that we can blame this on the desktop folks like that..
> after all i2c-slave was sufficient for embedded devices for some time,
> and it was an improvement over what came before when it comes to
> sharing external encoder support (ie. nothing)
>
> But, as was the case with atomic, the framework evolves.  And as
> atomic roll-out has shown, it doesn't require a flag-day if you accept
> some duplication.
>
>>> Anyways, if needed, just copy/paste the adv7511/7533 code into a
>>> separate bridge-only driver, and we'll use that.  Once the
>>> i2c-slave-encoder users for adv7511 are converted over, we can delete
>>> the original slave encoder driver.  That seems like a sane transition
>>> plan to a bridge-only world.
>>
>> It's a very sane way to make sure nobody will do the work and keep two copies
>> of the same code for a long time, yes.
>
> As opposed to a change everything at once approach, and corresponding
> updates to drivers for hw you don't have.  That sounds like a *much*
> saner plan</sarcasm>
>
> Seriously, the cost of duplicating a bit of code is low.  Especially
> when the code you are duplicating is low churn.  Duplicating lets
> other drivers that use adv75xx via slave-encoder migrate over at their
> own pace.  Similar to how atomic was rolled out.  To me that sounds
> like the much more practical way forward.
>
>> The path forward is pretty clear, the issue is to find someone who can do the
>> work.

I volunteer (as tribute)!

Once we settle on the best way to do it.

>
> Maybe the end goal is clear.  But apparently the path forward less so.

Laurent, as we'd discussed in the past, I did an initial study of how
we could map bridge ops to the encoder slave ops, and change it for the
rcar kms driver (and others) that might use adv7511.

The drm_encoder_slave_funcs ops are a superset of bridge ops. The 
rcar-du driver uses a subset of these ops (encoder-like ops) to
implement the hdmi encoder (rcar_du_hdmienc.c) and the other 
(connector-like ops)for implementing the hdmi connector
(rcar_du_hdmicon.c)

We have two options:

1. If we want to use drm_bridge as it is, the rcar driver shouldn't
create a hdmi connector anymore, and rely on the adv7511 driver to make
a connector for it. This is the easiest way, but it will break the nice
representation of the hardware in DT.

2. We add more drm_bridge ops (the ones that implement the connector 
ops), and make the hdmi connector created by rcar-du use these bridge
ops.

Both these options have issues. The 2) one is the probably the better of
the two, but I don't know if adding more ops to the bridge is the right
way to go.

Therefore, I don't know how we can solve this straight away. If there is
more debate needed on this, then it is probably easier to get adv7533
support in, and then figure out how two merge the two.

Archit

>
> BR,
> -R
>
>> --
>> Regards,
>>
>> Laurent Pinchart
>>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project