Hi Suraj
On 3/8/2022 6:30 AM, Kandpal, Suraj wrote:
Hi Abhinav,
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.
Seems like this patch series currently won't be able to move forward with the
connector pointer change so I guess you can split the series to encoder pointer
change where we optionally create encoder if required ,keeping my name in
co-developed tag so that msm can move forward with its implementation and
then we can work to see how the connector issue can be bypassed.
Best Regards,
Suraj Kandpal
Thanks, let me re-spin this and will keep your name as co-developer.
Should be able to get it on the list in a week.
Thanks
Abhinav
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.
