Thomas Zimmermann <tzimmerm...@suse.de> writes:
Hello Thomas,
> Hi Javier
>
> Am 23.03.25 um 11:57 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmerm...@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 <tzimmerm...@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 <javi...@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
