Re: [PATCH 08/18] drm/sysfb: Merge connector functions

[Javier Martinez Canillas <javierm@redhat.com>]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Hi Javier
>
> Am 23.03.25 um 11:57 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>
>>> Merge the connector functions of ofdrm and simpledrm. Replace the
>>> code in each driver with the shared helpers. Set up callbacks with
>>> initializer macros.
>>>
>>> No effective code changes. The sysfb connector only returns the
>>> preconfigured display mode.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>> ...
>>
>>> +#define DRM_SYSFB_CONNECTOR_FUNCS \
>>> +	.reset = drm_atomic_helper_connector_reset, \
>>> +	.fill_modes = drm_helper_probe_single_connector_modes, \
>>> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, \
>>> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state
>>> +
>>>   /*
>> ...
>>
>>>   static const struct drm_connector_funcs ofdrm_connector_funcs = {
>>> -	.reset = drm_atomic_helper_connector_reset,
>>> -	.fill_modes = drm_helper_probe_single_connector_modes,
>>> +	DRM_SYSFB_CONNECTOR_FUNCS,
>>>   	.destroy = drm_connector_cleanup,
>> Why not include the .destroy callback in DRM_SYSFB_CONNECTOR_FUNCS ?
>
> These sysfb helpers provide functionality to operate on the output 
> (damage handling, etc).
>
> The destroy callback depends on the way the mode-setting pipeline is 
> organized. The driver controls this. It might wants to allocated the 
> connector separately or use a container structure (e.g., struct 
> ofdrm_connector) that needs separate cleanup. Hence the driver has to 
> control the  destroy callback. That argument goes for all the other 
> elements of the pipeline.
>

Thanks for the explanation. Makes sense.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat