[PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes

[Abhinav Kumar]

Hi Suraj

On 3/4/2022 6:16 AM, Kandpal, Suraj wrote:
> Hi,
>>> Hi,
>>>>> Hi Abhinav,
>>>>>> Hi Laurent
>>>>>>
>>>>>> Ok sure, I can take this up but I need to understand the
>>>>>> proposal a little bit more before proceeding on this. So we will
>>>>>> discuss this in another email where we specifically talk about the
>> connector changes.
>>>>>>
>>>>>> Also, I am willing to take this up once the encoder part is done
>>>>>> and merged so that atleast we keep moving on that as MSM WB
>>>>>> implementation can proceed with that first.
>>>>>>
>>>>>> Hi Jani and Suraj
>>>>>>
>>>>>> Any concerns with splitting up the series into encoder and
>>>>>> connector OR re- arranging them?
>>>>>>
>>>>>> Let me know if you can do this OR I can also split this up
>>>>>> keeping Suraj's name in the Co-developed by tag.
>>>>> I was actually thinking of floating a new patch series with both
>>>>> encoder and connector pointer changes but with a change in
>>>>> initialization functions wherein we allocate a connector and
>>>>> encoder incase the driver doesn’t have its own this should
>>>>> minimize the effect on other drivers already using current drm
>>>>> writeback framework and also
>>>> allow i915 to create its own connector.
>>>>

I was proposing to split up the encoder and connector because the 
comments from Laurent meant we were in agreement about the encoder but 
not about the connector.

I am afraid another attempt with the similar idea is only going to stall 
the series again and hence i gave this option.

Eventually its upto Laurent and the other maintainers to guide us 
forward on this as this series has been stalled for weeks now.

>>>> I thought that Laurent was quite strict about the connector. So I'd
>>>> suggest targeting drm_writeback_connector with an optionally created
>>>> encoder and the builtin connector
>>> In that case even if we target that i915 will not be able to move
>>> forward with its implementation of writeback as builtin connector does
>>> not work for us right now as explained earlier, optionally creating
>>> encoder and connector will help everyone move forward and once done
>> we
>>> can actually sit and work on how to side step this issue using
>>> Laurent's suggestion
>>
>> This is up to Laurent & other DRM maintainers to decide whether this
>> approach would be accepted or not.
>> So far unfortunately I have mostly seen the pushback and a strong
>> suggestion to stop treating each drm_connector as i915_connector.
>> I haven't checked the exact details, but IMO adding a branch if the
>> connector's type is DRM_CONNECTOR_VIRTUAL should be doable.
> Bringing in the change where we don’t treat each drm_connector as
> an intel_connector or even adding a branch which checks if
> drm_connector is DRM_CONNECTOR_VIRTUAL and conditionally cast it
> to intel_connector is quite a lot of work for i915.
> That's why I was suggesting if for now we could move forward with optionally
> creating both encoder and connector minimizing affects on other drivers as
> well as allowing us to move forward.
> What do you think, Launrent?
> 
>>
>>>>
>>>>> We can work on Laurent's suggestion but that would first require
>>>>> the initial RFC patches and then a rework from both of our drivers
>>>>> side to see if they gel well with our current code which will take
>>>>> a considerable amount of time. We can for now however float the
>>>>> patch series up which minimally affects the current drivers and
>>>>> also allows
>>>>> i915 and msm to move forward with its writeback implementation off
>>>>> course with a proper documentation stating new drivers shouldn't
>>>>> try to
>>>> make their own connectors and encoders and that drm_writeback will
>>>> do that for them.
>>>>> Once that's done and merged we can work with Laurent on his
>>>>> proposal to improve the drm writeback framework so that this issue
>>>>> can be side
>>>> stepped entirely in future.
>>>>> For now I would like to keep the encoder and connector pointer
>>>>> changes
>>>> together.
>>>
>>
>>
>>
>> --
>> With best wishes
>> Dmitry
> 
> Best Regards,
> Suraj Kandpal