Re: [PATCH] drm/virtio: Open-code drm_simple_encoder_init()

[Dmitry Osipenko]

On 3/2/26 16:48, Thomas Zimmermann wrote:
> Hi
> 
> Am 27.02.26 um 11:35 schrieb Hardik Phalet:
>> drm_simple_encoder_init() is a thin wrapper around drm_encoder_init()
>> that only provides a minimal drm_encoder_funcs instance with
>> .destroy = drm_encoder_cleanup.
>>
>> Inline the helper in virtgpu_display.c and provide a local
>> drm_encoder_funcs instance instead. This removes the unnecessary
>> indirection and prepares for the eventual removal of
>> drm_simple_encoder_init().
>>
>> No functional changes intended.
>>
>> Signed-off-by: Hardik Phalet <hardik.phalet@xxxxx>
>> ---
>>   drivers/gpu/drm/virtio/virtgpu_display.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/
>> drm/virtio/virtgpu_display.c
>> index f1dae9569805..8bd6cdc6c16e 100644
>> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
>> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
>> @@ -232,6 +232,10 @@ static enum drm_mode_status
>> virtio_gpu_conn_mode_valid(struct drm_connector *con
>>       return MODE_BAD;
>>   }
>>   +static const struct drm_encoder_funcs virtio_gpu_enc_cleanup_funcs = {
>> +    .destroy = drm_encoder_cleanup
>> +};
>> +
>>   static const struct drm_encoder_helper_funcs
>> virtio_gpu_enc_helper_funcs = {
>>       .mode_set   = virtio_gpu_enc_mode_set,
>>       .enable     = virtio_gpu_enc_enable,
>> @@ -306,7 +310,8 @@ static int vgdev_output_init(struct
>> virtio_gpu_device *vgdev, int index)
>>       if (vgdev->has_edid)
>>           drm_connector_attach_edid_property(connector);
>>   -    drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_VIRTUAL);
>> +    drm_encoder_init(dev, encoder, &virtio_gpu_enc_cleanup_funcs,
>> +             DRM_MODE_ENCODER_VIRTUAL, NULL);
> 
> This looks correct, but you should also remove the include statement at [1]
> 
> [1] https://elixir.bootlin.com/linux/v6.19/source/drivers/gpu/drm/
> virtio/virtgpu_display.c#L35

The patch adds more lines than removes. What's wrong with
drm_simple_encoder_init() and why it needs to be removed eventually?

-- 
Best regards,
Dmitry