Re: [PATCH v2 1/6] drm/msm/mdp5: Use per-plane rotation property

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Oct 24, 2016 at 03:33:18PM +0530, Archit Taneja wrote:
> Hi Ville,
> 
> On 10/22/2016 12:52 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The global mode_config.rotation_property is going away, switch over to
> > per-plane rotation_property.
> 
> 
> I was trying to test this on msm/drm using modetest. The 180 rotation
> works fine, but drm rejects reflect-x and reflect-y rotation prop
> values. Is this expected?
> 
> I needed to make this modification to get reflect-x/y working too:
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a747bb1..9fcc2c9 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -711,7 +711,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>   		state->src_h = val;
>   	} else if (property == config->rotation_property ||
>   		   property == plane->rotation_property) {
> -		if (!is_power_of_2(val & DRM_ROTATE_MASK))
> +		if (!is_power_of_2(val & (DRM_ROTATE_MASK | DRM_REFLECT_MASK)))

That makes no sense. You _must_ to pass one and only one rotation angle.
In *addition* you can pass any number of the reflection flags.

>   			return -EINVAL;
>   		state->rotation = val;
>   	} else if (property == plane->zpos_property) {
> 
> 
> 
> Otherwise, the patches look fine to me.
> 
> Thanks,
> Archit
> 
> >
> > v2: Drop the BIT()
> >
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Jilai Wang <jilaiw@codeaurora.org>
> > Cc: Archit Taneja <architt@codeaurora.org>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Rob Clark <robdclark@gmail.com>
> > ---
> >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 14 +++++---------
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> > index 951c002b05df..2653ad893ebc 100644
> > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> > @@ -75,15 +75,11 @@ static void mdp5_plane_install_rotation_property(struct drm_device *dev,
> >  		!(mdp5_plane->caps & MDP_PIPE_CAP_VFLIP))
> >  		return;
> >
> > -	if (!dev->mode_config.rotation_property)
> > -		dev->mode_config.rotation_property =
> > -			drm_mode_create_rotation_property(dev,
> > -				DRM_ROTATE_0 | DRM_REFLECT_X | DRM_REFLECT_Y);
> > -
> > -	if (dev->mode_config.rotation_property)
> > -		drm_object_attach_property(&plane->base,
> > -			dev->mode_config.rotation_property,
> > -			DRM_ROTATE_0);
> > +	drm_plane_create_rotation_property(plane,
> > +					   DRM_ROTATE_0,
> > +					   DRM_ROTATE_0 |
> > +					   DRM_REFLECT_X |
> > +					   DRM_REFLECT_Y);
> >  }
> >
> >  /* helper to install properties which are common to planes and crtcs */
> >
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel