Re: [PATCH v2] drm: fix plane rotation when restoring fbdev configuration

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Aug 20, 2014 at 02:45:00PM +0100, Thomas Wood wrote:
> Make sure plane rotation is reset correctly when restoring the fbdev
> configuration by using drm_mode_plane_set_obj_prop which calls the
> driver's set_property callback.
> 
> The rotation reset feature was introduced in commit 9783de2 (drm:
> Resetting rotation property) and the callback issue was originally
> addressed in a previous version of the patch, but the fix was not
> present in the final version.
> 
> v2: Fix documentation warning
>     Add some more details to the commit message (Daniel Vetter)
> 
> Testcase: igt/kms_rotation_crc
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82236
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Thomas Wood <thomas.wood@intel.com>

Queued for -next, thanks for the patch.

Aside: If I've picked up a patch already, I prefer just the fixup diff to
squash in. I've kicked out your v1, but if I've frozen down my queue as
drm-intel-next already (or done a merge or something else that freezes
history) I need the fixup. And squashing in the fixup is not more work
than doing the rebase to kick out the old version anyway. And the fixup is
easier to review.

Thanks, Daniel
> ---
>  drivers/gpu/drm/drm_crtc.c      | 25 ++++++++++++++++++++-----
>  drivers/gpu/drm/drm_fb_helper.c |  6 +++---
>  include/drm/drm_crtc.h          |  3 +++
>  3 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f09b752..7d7c1fd 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -4175,12 +4175,25 @@ static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
>  	return ret;
>  }
>  
> -static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj,
> -				      struct drm_property *property,
> -				      uint64_t value)
> +/**
> + * drm_mode_plane_set_obj_prop - set the value of a property
> + * @plane: drm plane object to set property value for
> + * @property: property to set
> + * @value: value the property should be set to
> + *
> + * This functions sets a given property on a given plane object. This function
> + * calls the driver's ->set_property callback and changes the software state of
> + * the property if the callback succeeds.
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
> +				struct drm_property *property,
> +				uint64_t value)
>  {
>  	int ret = -EINVAL;
> -	struct drm_plane *plane = obj_to_plane(obj);
> +	struct drm_mode_object *obj = &plane->base;
>  
>  	if (plane->funcs->set_property)
>  		ret = plane->funcs->set_property(plane, property, value);
> @@ -4189,6 +4202,7 @@ static int drm_mode_plane_set_obj_prop(struct drm_mode_object *obj,
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL(drm_mode_plane_set_obj_prop);
>  
>  /**
>   * drm_mode_getproperty_ioctl - get the current value of a object's property
> @@ -4327,7 +4341,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
>  		ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value);
>  		break;
>  	case DRM_MODE_OBJECT_PLANE:
> -		ret = drm_mode_plane_set_obj_prop(arg_obj, property, arg->value);
> +		ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj),
> +						  property, arg->value);
>  		break;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 63d7b8e..0c0c39b 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -296,9 +296,9 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>  			drm_plane_force_disable(plane);
>  
>  		if (dev->mode_config.rotation_property) {
> -			drm_object_property_set_value(&plane->base,
> -					dev->mode_config.rotation_property,
> -					BIT(DRM_ROTATE_0));
> +			drm_mode_plane_set_obj_prop(plane,
> +						    dev->mode_config.rotation_property,
> +						    BIT(DRM_ROTATE_0));
>  		}
>  	}
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 0375d75..31344bf 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1127,6 +1127,9 @@ extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
>  					     struct drm_file *file_priv);
>  extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
>  					   struct drm_file *file_priv);
> +extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
> +				       struct drm_property *property,
> +				       uint64_t value);
>  
>  extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
>  				 int *bpp);
> -- 
> 1.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch