Re: [PATCH 5/7] drm/i915: Use & instead if == to check for rotations

Joonas Lahtinen
2016-07-20T14:01:00Z

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Using == to check for 180 degree rotation only works as long as the
> reflection bits aren't set. That will change soon enough for CHV, so
> let's stop doing things the wrong way.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reading through the patch makes me wanna do the change;

#define DRM_ROTATE_180 BIT(x)

even more. Maybe with coccinelle.

Regards, Joonas

> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++----
>  drivers/gpu/drm/i915/intel_sprite.c  | 6 +++---
>  2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 93ecb259c5ce..79c1a8b89d1d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2688,7 +2688,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>   intel_crtc->dspaddr_offset = linear_offset;
>   }
>
> - if (rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation & BIT(DRM_ROTATE_180)) {
>   dspcntr |= DISPPLANE_ROTATE_180;
>
>   x += (crtc_state->pipe_src_w - 1);
> @@ -2791,7 +2791,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
>   intel_compute_tile_offset(&x, &y, fb, 0,
>     fb->pitches[0], rotation);
>   linear_offset -= intel_crtc->dspaddr_offset;
> - if (rotation == BIT(DRM_ROTATE_180)) {
> +
> + if (rotation & BIT(DRM_ROTATE_180)) {
>   dspcntr |= DISPPLANE_ROTATE_180;
>
>   if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> @@ -10263,7 +10264,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
>   if (HAS_DDI(dev))
>   cntl |= CURSOR_PIPE_CSC_ENABLE;
>
> - if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
> + if (plane_state->base.rotation & BIT(DRM_ROTATE_180))
>   cntl |= CURSOR_ROTATE_180;
>   }
>
> @@ -10309,7 +10310,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>
>   /* ILK+ do this automagically */
>   if (HAS_GMCH_DISPLAY(dev) &&
> -     plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> +     plane_state->base.rotation & BIT(DRM_ROTATE_180)) {
>   base += (plane_state->base.crtc_h *
>    plane_state->base.crtc_w - 1) * 4;
>   }
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index bc41c1bba04c..6b815d57d75a 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -444,7 +444,7 @@ vlv_update_plane(struct drm_plane *dplane,
>      fb->pitches[0], rotation);
>   linear_offset -= sprsurf_offset;
>
> - if (rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation & BIT(DRM_ROTATE_180)) {
>   sprctl |= SP_ROTATE_180;
>
>   x += src_w;
> @@ -577,7 +577,7 @@ ivb_update_plane(struct drm_plane *plane,
>      fb->pitches[0], rotation);
>   linear_offset -= sprsurf_offset;
>
> - if (rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation & BIT(DRM_ROTATE_180)) {
>   sprctl |= SPRITE_ROTATE_180;
>
>   /* HSW and BDW does this automagically in hardware */
> @@ -714,7 +714,7 @@ ilk_update_plane(struct drm_plane *plane,
>      fb->pitches[0], rotation);
>   linear_offset -= dvssurf_offset;
>
> - if (rotation == BIT(DRM_ROTATE_180)) {
> + if (rotation & BIT(DRM_ROTATE_180)) {
>   dvscntr |= DVS_ROTATE_180;
>
>   x += src_w;
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation