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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Suraj

On 2/22/2022 10:17 PM, Kandpal, Suraj wrote:
Hey,

The connector/encoder funcs you do have to pass to
drm_writeback_connector_init() can't use any of the shared driver
infrastructure that assume a certain inheritance.

See also my reply to Laurent [1].

It well might be that we all misunderstand your design. Do you have a
complete intel drm_writeback implementation based on this patchset? It
would be easier to judge if the approach is correct seeing your
target.

That would be up to Suraj Kandpal.
I have floated the intel drm_writeback implementation.
Please refer to [1] to get a better understanding of how we are implementing
writeback functionality.

[1] https://patchwork.freedesktop.org/series/100617/

Thanks,
Suraj Kandpal

Based on the discussion in this thread [1] , it seems like having drm_encoder as a pointer seems to have merits for both of us and also in agreement with the folks on this thread and has a better chance of an ack.

However drm_connector is not.

I had a brief look at your implementation. Any reason you need to go through intel_connector here and not drm_writeback_connector directly?

The reason I ask is that I see you are not using prepare_writeback_job hook. If you use that, you can use the drm_writeback_connector passed on from there instead of looping through your list like you are doing in intel_find_writeback_connector.

Also, none of the other entries of struct intel_connector are being used for the writeback implementation. So does the drm_writeback_connector in your implementation need to be an intel_connector when its anyway not using other fields? Why cant it be just stored as a drm_writeback_connector itself in your struct intel_wd.

@@ -539,6 +540,8 @@  struct intel_connector {
 	struct work_struct modeset_retry_work;

 	struct intel_hdcp hdcp;
+
+	struct drm_writeback_connector wb_conn;
 };

[1] https://patchwork.kernel.org/project/dri-devel/patch/20220202085429.22261-6-suraj.kandpal@xxxxxxxxx/#24747889

If you are in agreement with this, do you think you can re-spin the series only with the drm_encoder as a pointer without the drm_connector part.



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux