Re: [PATCH v3 4/4] drm/qxl: Use simple encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sam

Am 27.02.20 um 21:45 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Tue, Feb 25, 2020 at 02:10:55PM +0100, Thomas Zimmermann wrote:
>> The qxl driver uses an empty implementation for its encoder. Replace
>> the code with the generic simple encoder.
>>
>> v2:
>> 	* rebase onto new simple-encoder interface
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
>> Acked-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
>> ---
>>  drivers/gpu/drm/qxl/qxl_display.c | 18 +++---------------
>>  1 file changed, 3 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
>> index ab4f8dd00400..9c0e1add59fb 100644
>> --- a/drivers/gpu/drm/qxl/qxl_display.c
>> +++ b/drivers/gpu/drm/qxl/qxl_display.c
>> @@ -31,6 +31,7 @@
>>  #include <drm/drm_gem_framebuffer_helper.h>
>>  #include <drm/drm_plane_helper.h>
>>  #include <drm/drm_probe_helper.h>
>> +#include <drm/drm_simple_kms_helper.h>
>>  
>>  #include "qxl_drv.h"
>>  #include "qxl_object.h"
>> @@ -1007,9 +1008,6 @@ static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector)
>>  	return &qxl_output->enc;
>>  }
>>  
>> -static const struct drm_encoder_helper_funcs qxl_enc_helper_funcs = {
>> -};
>> -
>>  static const struct drm_connector_helper_funcs qxl_connector_helper_funcs = {
>>  	.get_modes = qxl_conn_get_modes,
>>  	.mode_valid = qxl_conn_mode_valid,
>> @@ -1059,15 +1057,6 @@ static const struct drm_connector_funcs qxl_connector_funcs = {
>>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>  };
>>  
>> -static void qxl_enc_destroy(struct drm_encoder *encoder)
>> -{
>> -	drm_encoder_cleanup(encoder);
>> -}
>> -
>> -static const struct drm_encoder_funcs qxl_enc_funcs = {
>> -	.destroy = qxl_enc_destroy,
>> -};
>> -
>>  static int qxl_mode_create_hotplug_mode_update_property(struct qxl_device *qdev)
>>  {
>>  	if (qdev->hotplug_mode_update_property)
>> @@ -1098,15 +1087,14 @@ static int qdev_output_init(struct drm_device *dev, int num_output)
>>  	drm_connector_init(dev, &qxl_output->base,
>>  			   &qxl_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
>>  
>> -	drm_encoder_init(dev, &qxl_output->enc, &qxl_enc_funcs,
>> -			 DRM_MODE_ENCODER_VIRTUAL, NULL);
>> +	drm_simple_encoder_init(dev, &qxl_output->enc,
>> +				DRM_MODE_ENCODER_VIRTUAL);
> return value is ignored - as it was before.
> A quick grep told that it is 50/50 if return value is checked.
> So OK.

Well, now that you bring it up, I rather test for the returned value.
Continuing without an initialized encoder doesn't seem to make much sense.

Thanks for reviewing the patchset. I'll address the comments you had and
post another update before merging.

Best regards
Thomas

> 
> Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
>>  
>>  	/* we get HPD via client monitors config */
>>  	connector->polled = DRM_CONNECTOR_POLL_HPD;
>>  	encoder->possible_crtcs = 1 << num_output;
>>  	drm_connector_attach_encoder(&qxl_output->base,
>>  					  &qxl_output->enc);
>> -	drm_encoder_helper_add(encoder, &qxl_enc_helper_funcs);
>>  	drm_connector_helper_add(connector, &qxl_connector_helper_funcs);
>>  
>>  	drm_object_attach_property(&connector->base,
>> -- 
>> 2.25.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]