[Intel-gfx] [PATCH 1/2] drm/i915: Move 90/270 rotation validity check into its own function

2018-08-27 Thread Juha-Pekka Heikkila 
This makes intel_plane_atomic_check_with_state() generally shorter.

v2: (Ville Syrjälä) move all rotation related checks into new function and
don't pass dev_priv pointer around.

v3: (Ville Syljälä) rename new function.

v4: rebase

Signed-off-by: Juha-Pekka Heikkila 
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 66 ++-
 1 file changed, 39 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index dcba645..344a16b 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -107,44 +107,34 @@ intel_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
 }
 
-int intel_plane_atomic_check_with_state(const struct intel_crtc_state 
*old_crtc_state,
-   struct intel_crtc_state *crtc_state,
-   const struct intel_plane_state 
*old_plane_state,
-   struct intel_plane_state *intel_state)
+static bool intel_plane_valid_rotation(const struct drm_plane_state 
*plane_state)
 {
-   struct drm_plane *plane = intel_state->base.plane;
-   struct drm_i915_private *dev_priv = to_i915(plane->dev);
-   struct drm_plane_state *state = _state->base;
-   struct intel_plane *intel_plane = to_intel_plane(plane);
-   const struct drm_display_mode *adjusted_mode =
-   _state->base.adjusted_mode;
-   int ret;
+   struct intel_plane *plane = to_intel_plane(plane_state->plane);
+   struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 
-   if (!intel_state->base.crtc && !old_plane_state->base.crtc)
-   return 0;
-
-   if (state->fb && drm_rotation_90_or_270(state->rotation)) {
+   if (plane_state->fb &&
+   drm_rotation_90_or_270(plane_state->rotation)) {
struct drm_format_name_buf format_name;
 
-   if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
-   state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
+   if (plane_state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
+   plane_state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
-   return -EINVAL;
+   return false;
}
 
/*
 * 90/270 is not allowed with RGB64 16:16:16:16,
 * RGB 16-bit 5:6:5, and Indexed 8-bit.
-* TBD: Add RGB64 case once its added in supported format list.
+* TBD: Add RGB64 case once its added in supported format
+* list.
 */
-   switch (state->fb->format->format) {
+   switch (plane_state->fb->format->format) {
case DRM_FORMAT_C8:
case DRM_FORMAT_RGB565:
DRM_DEBUG_KMS("Unsupported pixel format %s for 
90/270!\n",
- 
drm_get_format_name(state->fb->format->format,
- _name));
-   return -EINVAL;
-
+ 
drm_get_format_name(plane_state->fb->format->format,
+ _name));
+   return false;
default:
break;
}
@@ -152,12 +142,34 @@ int intel_plane_atomic_check_with_state(const struct 
intel_crtc_state *old_crtc_
 
/* CHV ignores the mirror bit when the rotate bit is set :( */
if (IS_CHERRYVIEW(dev_priv) &&
-   state->rotation & DRM_MODE_ROTATE_180 &&
-   state->rotation & DRM_MODE_REFLECT_X) {
+   plane_state->rotation & DRM_MODE_ROTATE_180 &&
+   plane_state->rotation & DRM_MODE_REFLECT_X) {
DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
-   return -EINVAL;
+   return false;
}
 
+   return true;
+}
+
+int intel_plane_atomic_check_with_state(const struct intel_crtc_state 
*old_crtc_state,
+   struct intel_crtc_state *crtc_state,
+   const struct intel_plane_state 
*old_plane_state,
+   struct intel_plane_state *intel_state)
+{
+   struct drm_plane *plane = intel_state->base.plane;
+   struct drm_i915_private *dev_priv = to_i915(plane->dev);
+   struct drm_plane_state *state = _state->base;
+   struct intel_plane *intel_plane = to_intel_plane(plane);
+   const struct drm_display_mode *adjusted_mode =
+   _state->base.adjusted_mode;
+   int ret;
+
+   if (!intel_state->base.crtc && !old_plane_state->base.crtc)
+   return 0;
+
+   if

[Intel-gfx] [PATCH 1/2] drm/i915: Move 90/270 rotation validity check into its own function

2017-11-21 Thread Juha-Pekka Heikkila 
This makes intel_plane_atomic_check_with_state() generally shorter.

v2: (Ville Syrjälä) move all rotation related checks into new function and 
don't pass dev_priv pointer around.

v3: (Ville Syljälä) rebase and small detail changes.

Signed-off-by: Juha-Pekka Heikkila 
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 78 ++-
 1 file changed, 45 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 8e6dc15..8be50309 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -107,6 +107,50 @@ intel_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
 }
 
+static bool intel_plane_valid_rotation(const struct drm_plane_state 
*plane_state)
+{
+   struct intel_plane *plane = to_intel_plane(plane_state->plane);
+   struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+
+   if (plane_state->fb &&
+   drm_rotation_90_or_270(plane_state->rotation)) {
+   struct drm_format_name_buf format_name;
+
+   if (plane_state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
+   plane_state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
+   DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
+   return false;
+   }
+
+   /*
+* 90/270 is not allowed with RGB64 16:16:16:16,
+* RGB 16-bit 5:6:5, and Indexed 8-bit.
+* TBD: Add RGB64 case once its added in supported format
+* list.
+*/
+   switch (plane_state->fb->format->format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   DRM_DEBUG_KMS("Unsupported pixel format %s for 
90/270!\n",
+ 
drm_get_format_name(plane_state->fb->format->format,
+ _name));
+   return false;
+   default:
+   break;
+   }
+   }
+
+   /* CHV ignores the mirror bit when the rotate bit is set :( */
+   if (IS_CHERRYVIEW(dev_priv) &&
+   plane_state->rotation & DRM_MODE_ROTATE_180 &&
+   plane_state->rotation & DRM_MODE_REFLECT_X) {
+   DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
+   return false;
+   }
+
+   return true;
+}
+
 int intel_plane_atomic_check_with_state(const struct intel_crtc_state 
*old_crtc_state,
struct intel_crtc_state *crtc_state,
const struct intel_plane_state 
*old_plane_state,
@@ -137,40 +181,8 @@ int intel_plane_atomic_check_with_state(const struct 
intel_crtc_state *old_crtc_
intel_state->clip.y2 =
crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
 
-   if (state->fb && drm_rotation_90_or_270(state->rotation)) {
-   struct drm_format_name_buf format_name;
-
-   if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
-   state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
-   DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
-   return -EINVAL;
-   }
-
-   /*
-* 90/270 is not allowed with RGB64 16:16:16:16,
-* RGB 16-bit 5:6:5, and Indexed 8-bit.
-* TBD: Add RGB64 case once its added in supported format list.
-*/
-   switch (state->fb->format->format) {
-   case DRM_FORMAT_C8:
-   case DRM_FORMAT_RGB565:
-   DRM_DEBUG_KMS("Unsupported pixel format %s for 
90/270!\n",
- 
drm_get_format_name(state->fb->format->format,
- _name));
-   return -EINVAL;
-
-   default:
-   break;
-   }
-   }
-
-   /* CHV ignores the mirror bit when the rotate bit is set :( */
-   if (IS_CHERRYVIEW(dev_priv) &&
-   state->rotation & DRM_MODE_ROTATE_180 &&
-   state->rotation & DRM_MODE_REFLECT_X) {
-   DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
+   if (!intel_plane_valid_rotation(state))
return -EINVAL;
-   }
 
intel_state->base.visible = false;
ret = intel_plane->check_plane(intel_plane, crtc_state, intel_state);
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Move 90/270 rotation validity check into its own function

2017-11-03 Thread Ville Syrjälä 
On Fri, Nov 03, 2017 at 05:50:17PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 03, 2017 at 04:37:57PM +0200, Juha-Pekka Heikkila wrote:
> > This makes intel_plane_atomic_check_with_state() generally shorter.
> > 
> > Signed-off-by: Juha-Pekka Heikkila 
> > ---
> >  drivers/gpu/drm/i915/intel_atomic_plane.c | 53 
> > +--
> >  1 file changed, 30 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
> > b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index 8e6dc15..6c4c82e2d 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -107,6 +107,35 @@ intel_plane_destroy_state(struct drm_plane *plane,
> > drm_atomic_helper_plane_destroy_state(plane, state);
> >  }
> >  
> > +static bool intel_valid_rotation(const struct drm_plane_state *state)

Oh and please name this parameter 'plane_state'. We're trying to slowly
clean up the mess with inconsistent naming of things.

> > +{
> > +   struct drm_format_name_buf format_name;
> > +
> > +   if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> > +   state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> > +   DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> > +   return false;
> > +   }
> > +
> > +   /*
> > +* 90/270 is not allowed with RGB64 16:16:16:16,
> > +* RGB 16-bit 5:6:5, and Indexed 8-bit.
> > +* TBD: Add RGB64 case once its added in supported format list.
> > +*/
> > +   switch (state->fb->format->format) {
> > +   case DRM_FORMAT_C8:
> > +   case DRM_FORMAT_RGB565:
> > +   DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
> > + drm_get_format_name(state->fb->format->format,
> > + _name));
> > +   return false;
> > +
> > +   default:
> > +   break;
> > +   }
> 
> Usually there's an empty line after the final return.
> 
> > +   return true;
> > +}
> > +
> >  int intel_plane_atomic_check_with_state(const struct intel_crtc_state 
> > *old_crtc_state,
> > struct intel_crtc_state *crtc_state,
> > const struct intel_plane_state 
> > *old_plane_state,
> > @@ -138,30 +167,8 @@ int intel_plane_atomic_check_with_state(const struct 
> > intel_crtc_state *old_crtc_
> > crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
> >  
> > if (state->fb && drm_rotation_90_or_270(state->rotation)) {
> 
> I'd pull these checks into the new function as well (as an early
> return so that we don't have to needlessly indent the whole
> function body). Otherwise the function name doesn't really match
> the implementation.
> 
> > -   struct drm_format_name_buf format_name;
> > -
> > -   if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> > -   state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> > -   DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> > +   if (!intel_valid_rotation(state))
> > return -EINVAL;
> > -   }
> > -
> > -   /*
> > -* 90/270 is not allowed with RGB64 16:16:16:16,
> > -* RGB 16-bit 5:6:5, and Indexed 8-bit.
> > -* TBD: Add RGB64 case once its added in supported format list.
> > -*/
> > -   switch (state->fb->format->format) {
> > -   case DRM_FORMAT_C8:
> > -   case DRM_FORMAT_RGB565:
> > -   DRM_DEBUG_KMS("Unsupported pixel format %s for 
> > 90/270!\n",
> > - 
> > drm_get_format_name(state->fb->format->format,
> > - _name));
> > -   return -EINVAL;
> > -
> > -   default:
> > -   break;
> > -   }
> > }
> >  
> > /* CHV ignores the mirror bit when the rotate bit is set :( */
> > -- 
> > 2.7.4
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Move 90/270 rotation validity check into its own function

2017-11-03 Thread Ville Syrjälä 
On Fri, Nov 03, 2017 at 04:37:57PM +0200, Juha-Pekka Heikkila wrote:
> This makes intel_plane_atomic_check_with_state() generally shorter.
> 
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 53 
> +--
>  1 file changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 8e6dc15..6c4c82e2d 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -107,6 +107,35 @@ intel_plane_destroy_state(struct drm_plane *plane,
>   drm_atomic_helper_plane_destroy_state(plane, state);
>  }
>  
> +static bool intel_valid_rotation(const struct drm_plane_state *state)
> +{
> + struct drm_format_name_buf format_name;
> +
> + if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> + state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> + DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> + return false;
> + }
> +
> + /*
> +  * 90/270 is not allowed with RGB64 16:16:16:16,
> +  * RGB 16-bit 5:6:5, and Indexed 8-bit.
> +  * TBD: Add RGB64 case once its added in supported format list.
> +  */
> + switch (state->fb->format->format) {
> + case DRM_FORMAT_C8:
> + case DRM_FORMAT_RGB565:
> + DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
> +   drm_get_format_name(state->fb->format->format,
> +   _name));
> + return false;
> +
> + default:
> + break;
> + }

Usually there's an empty line after the final return.

> + return true;
> +}
> +
>  int intel_plane_atomic_check_with_state(const struct intel_crtc_state 
> *old_crtc_state,
>   struct intel_crtc_state *crtc_state,
>   const struct intel_plane_state 
> *old_plane_state,
> @@ -138,30 +167,8 @@ int intel_plane_atomic_check_with_state(const struct 
> intel_crtc_state *old_crtc_
>   crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
>  
>   if (state->fb && drm_rotation_90_or_270(state->rotation)) {

I'd pull these checks into the new function as well (as an early
return so that we don't have to needlessly indent the whole
function body). Otherwise the function name doesn't really match
the implementation.

> - struct drm_format_name_buf format_name;
> -
> - if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
> - state->fb->modifier != I915_FORMAT_MOD_Yf_TILED) {
> - DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> + if (!intel_valid_rotation(state))
>   return -EINVAL;
> - }
> -
> - /*
> -  * 90/270 is not allowed with RGB64 16:16:16:16,
> -  * RGB 16-bit 5:6:5, and Indexed 8-bit.
> -  * TBD: Add RGB64 case once its added in supported format list.
> -  */
> - switch (state->fb->format->format) {
> - case DRM_FORMAT_C8:
> - case DRM_FORMAT_RGB565:
> - DRM_DEBUG_KMS("Unsupported pixel format %s for 
> 90/270!\n",
> -   
> drm_get_format_name(state->fb->format->format,
> -   _name));
> - return -EINVAL;
> -
> - default:
> - break;
> - }
>   }
>  
>   /* CHV ignores the mirror bit when the rotate bit is set :( */
> -- 
> 2.7.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx