Re: [Intel-gfx] [PATCH 1/4] drm: add writeback pointers to drm_connector

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 06 Jan 2022, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> > Adding drm_connector and drm_encoder pointers in
>> > drm_writeback_connector
>> 
>> Why?
>
> The elements of struct drm_writeback_connector are 
> struct drm_writeback_connector {
> 	Struct drm_connector base;
> 	Struct drm_encoder encoder;
>
> Similarly the elements of intel_encoder and intel_connector are
>  struct intel_encoder {
> 	Struct drm_encoder base;
>
> Struct intel_connector {
> 	Struct drm_connector base;
>
> The function drm_writeback_connector_init() will initialize the drm_connector and drm_encoder and attach them as well.
> Since the drm_connector/encoder are both struct in drm_writeback_connector and intel_connector/encoder, one of them should be a pointer, else both will be pointing to 2 separate instances.
>
> Usually the struct defined in drm framework pointing to any struct will be pointer and allocating them and initialization will be done with the users.
> Like struct drm_connector and drm_encoder are part of drm framework and the users of these such as i915 have included them in their struct intel_connector and intel_encoder. 
> Likewise struct drm_writeback_connector is a special connector and hence is not a user of drm_connector and hence this should be pointers.

Okay, I can see the problem, but the solution you present breaks at
least komeda, mali, rcard-du, vc4, and vkms drivers. Either you need to
change all of them, or figure out another way. I don't know off the top
of my head what the correct answer should be, but you can't break other
drivers. Again, maybe you'll get better ideas with a bigger audience
than just intel-gfx.


BR,
Jani.



>> 
>> We can all read the code, the commit message should mostly be about the
>> *why*.
>> 
>> Also, drm changes should Cc: dri-devel mailing list.
> Sure we will add this in Cc in the next series of patches
>
> Thanks,
> Suraj Kandpal

-- 
Jani Nikula, Intel Open Source Graphics Center