On Thu, 06 Jan 2022, "Kandpal, Suraj" <suraj.kandpal@xxxxxxxxx> 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
