Re: [PATCH 6/9] drm: Add drm_rotation_simplify()

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

On Mon, Oct 14, 2013 at 04:46:50PM +0300, Imre Deak wrote:
> On Mon, 2013-09-30 at 17:44 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > drm_rotation_simplify() can be used to eliminate unsupported rotation
> > flags. It will check if any unsupported flags are present, and if so
> > it will modify the rotation to an alternate form by adding 180 degrees
> > to rotation angle, and flipping the reflect x and y bits. The hope is
> > that this identity transform will eliminate the unsupported flags.
> > 
> > Of course that might not result in any more supported rotation, so
> > the caller is still responsible for checking the result afterwards.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_crtc.c | 30 ++++++++++++++++++++++++++++++
> >  include/drm/drm_crtc.h     |  2 ++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 0218681..665b807 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -3976,6 +3976,36 @@ int drm_format_vert_chroma_subsampling(uint32_t format)
> >  EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
> >  
> >  /**
> > + * drm_rotation_simplify() - Try to simplify the rotation
> > + * @rotation: Rotation to be simplified
> > + * @supported_rotations: Supported rotations
> > + *
> > + * Attempt to simplify the rotation to a form that is supported.
> > + * Eg. if the hardware supports everything except DRM_REFLECT_X
> > + * one could call this function like this:
> > + *
> > + * drm_rotation_simplify(rotation, BIT(DRM_ROTATE_0) |
> > + *                       BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_180) |
> > + *                       BIT(DRM_ROTATE_270) | BIT(DRM_REFLECT_Y));
> > + *
> > + * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
>                       ^ DRM_REFLECT_X
> 
> You don't use this function in your patchset, though
> intel_plane_set_property() could use it by accepting REFLECT_X|REFLECT_Y
> too. But it does what it says, so I'm also ok if you leave it unused for
> now.

Yeah I was going back and forth whether I should accept the reflect
flags, but in the end decided against it since we can only support
a limited combination of the flags. But the function could be useful
for someone else.

> 
> --Imre
> 
> > + * transforms the hardware supports, this function may not
> > + * be able to produce a supported transform, so the caller should
> > + * check the result afterwards.
> > + */
> > +unsigned int drm_rotation_simplify(unsigned int rotation,
> > +				   unsigned int supported_rotations)
> > +{
> > +	if (rotation & ~supported_rotations) {
> > +		rotation ^= BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y);
> > +		rotation = (rotation & ~0xf) | BIT((ffs(rotation & 0xf) + 1) % 4);
> > +	}
> > +
> > +	return rotation;
> > +}
> > +EXPORT_SYMBOL(drm_rotation_simplify);
> > +
> > +/**
> >   * drm_mode_config_init - initialize DRM mode_configuration structure
> >   * @dev: DRM device
> >   *
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index d25ba84..b97b367 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -1154,5 +1154,7 @@ extern int drm_format_vert_chroma_subsampling(uint32_t format);
> >  extern const char *drm_get_format_name(uint32_t format);
> >  extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> >  							      unsigned int supported_rotations);
> > +extern unsigned int drm_rotation_simplify(unsigned int rotation,
> > +					  unsigned int supported_rotations);
> >  
> >  #endif /* __DRM_CRTC_H__ */
> 



-- 
Ville Syrjälä
Intel OTC