Re: [Intel-gfx] [PATCH 14/14] drm/i915: Nuke pointless middle men for skl+ plane programming

[Souza, Jose]

On Wed, 2021-12-01 at 17:25 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> There is no real point in having this two stage
> skl_program_plane*() vs. skl_plane_update*() wrapper stuff.
> All we need to do is determine the correct color plane and
> we're done.

Reviewed-by: José Roberto de Souza 

> 
> Signed-off-by: Ville Syrjälä 
> ---
>  .../drm/i915/display/skl_universal_plane.c| 53 ++-
>  1 file changed, 17 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c 
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 92270679a99c..de2708ac1802 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1053,15 +1053,24 @@ static void icl_plane_csc_load_black(struct 
> intel_plane *plane)
>   intel_de_write_fw(i915, PLANE_CSC_POSTOFF(pipe, plane_id, 2), 0);
>  }
>  
> +static int skl_plane_color_plane(const struct intel_plane_state *plane_state)
> +{
> + /* Program the UV plane on planar master */
> + if (plane_state->planar_linked_plane && !plane_state->planar_slave)
> + return 1;
> + else
> + return 0;
> +}
> +
>  static void
> -skl_program_plane_noarm(struct intel_plane *plane,
> - const struct intel_crtc_state *crtc_state,
> - const struct intel_plane_state *plane_state,
> - int color_plane)
> +skl_plane_update_noarm(struct intel_plane *plane,
> +const struct intel_crtc_state *crtc_state,
> +const struct intel_plane_state *plane_state)
>  {
>   struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>   enum plane_id plane_id = plane->id;
>   enum pipe pipe = plane->pipe;
> + int color_plane = skl_plane_color_plane(plane_state);
>   u32 stride = skl_plane_stride(plane_state, color_plane);
>   const struct drm_framebuffer *fb = plane_state->hw.fb;
>   int crtc_x = plane_state->uapi.dst.x1;
> @@ -1114,14 +1123,14 @@ skl_program_plane_noarm(struct intel_plane *plane,
>  }
>  
>  static void
> -skl_program_plane_arm(struct intel_plane *plane,
> -   const struct intel_crtc_state *crtc_state,
> -   const struct intel_plane_state *plane_state,
> -   int color_plane)
> +skl_plane_update_arm(struct intel_plane *plane,
> +  const struct intel_crtc_state *crtc_state,
> +  const struct intel_plane_state *plane_state)
>  {
>   struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>   enum plane_id plane_id = plane->id;
>   enum pipe pipe = plane->pipe;
> + int color_plane = skl_plane_color_plane(plane_state);
>   u32 x = plane_state->view.color_plane[color_plane].x;
>   u32 y = plane_state->view.color_plane[color_plane].y;
>   u32 plane_color_ctl = 0;
> @@ -1202,34 +1211,6 @@ skl_plane_async_flip(struct intel_plane *plane,
>   spin_unlock_irqrestore(_priv->uncore.lock, irqflags);
>  }
>  
> -static void
> -skl_plane_update_noarm(struct intel_plane *plane,
> -const struct intel_crtc_state *crtc_state,
> -const struct intel_plane_state *plane_state)
> -{
> - int color_plane = 0;
> -
> - if (plane_state->planar_linked_plane && !plane_state->planar_slave)
> - /* Program the UV plane on planar master */
> - color_plane = 1;
> -
> - skl_program_plane_noarm(plane, crtc_state, plane_state, color_plane);
> -}
> -
> -static void
> -skl_plane_update_arm(struct intel_plane *plane,
> -  const struct intel_crtc_state *crtc_state,
> -  const struct intel_plane_state *plane_state)
> -{
> - int color_plane = 0;
> -
> - if (plane_state->planar_linked_plane && !plane_state->planar_slave)
> - /* Program the UV plane on planar master */
> - color_plane = 1;
> -
> - skl_program_plane_arm(plane, crtc_state, plane_state, color_plane);
> -}
> -
>  static bool intel_format_is_p01x(u32 format)
>  {
>   switch (format) {

[Intel-gfx] [PATCH 14/14] drm/i915: Nuke pointless middle men for skl+ plane programming

[Ville Syrjala]

From: Ville Syrjälä 

There is no real point in having this two stage
skl_program_plane*() vs. skl_plane_update*() wrapper stuff.
All we need to do is determine the correct color plane and
we're done.

Signed-off-by: Ville Syrjälä 
---
 .../drm/i915/display/skl_universal_plane.c| 53 ++-
 1 file changed, 17 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c 
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 92270679a99c..de2708ac1802 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1053,15 +1053,24 @@ static void icl_plane_csc_load_black(struct intel_plane 
*plane)
intel_de_write_fw(i915, PLANE_CSC_POSTOFF(pipe, plane_id, 2), 0);
 }
 
+static int skl_plane_color_plane(const struct intel_plane_state *plane_state)
+{
+   /* Program the UV plane on planar master */
+   if (plane_state->planar_linked_plane && !plane_state->planar_slave)
+   return 1;
+   else
+   return 0;
+}
+
 static void
-skl_program_plane_noarm(struct intel_plane *plane,
-   const struct intel_crtc_state *crtc_state,
-   const struct intel_plane_state *plane_state,
-   int color_plane)
+skl_plane_update_noarm(struct intel_plane *plane,
+  const struct intel_crtc_state *crtc_state,
+  const struct intel_plane_state *plane_state)
 {
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
+   int color_plane = skl_plane_color_plane(plane_state);
u32 stride = skl_plane_stride(plane_state, color_plane);
const struct drm_framebuffer *fb = plane_state->hw.fb;
int crtc_x = plane_state->uapi.dst.x1;
@@ -1114,14 +1123,14 @@ skl_program_plane_noarm(struct intel_plane *plane,
 }
 
 static void
-skl_program_plane_arm(struct intel_plane *plane,
- const struct intel_crtc_state *crtc_state,
- const struct intel_plane_state *plane_state,
- int color_plane)
+skl_plane_update_arm(struct intel_plane *plane,
+const struct intel_crtc_state *crtc_state,
+const struct intel_plane_state *plane_state)
 {
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
+   int color_plane = skl_plane_color_plane(plane_state);
u32 x = plane_state->view.color_plane[color_plane].x;
u32 y = plane_state->view.color_plane[color_plane].y;
u32 plane_color_ctl = 0;
@@ -1202,34 +1211,6 @@ skl_plane_async_flip(struct intel_plane *plane,
spin_unlock_irqrestore(_priv->uncore.lock, irqflags);
 }
 
-static void
-skl_plane_update_noarm(struct intel_plane *plane,
-  const struct intel_crtc_state *crtc_state,
-  const struct intel_plane_state *plane_state)
-{
-   int color_plane = 0;
-
-   if (plane_state->planar_linked_plane && !plane_state->planar_slave)
-   /* Program the UV plane on planar master */
-   color_plane = 1;
-
-   skl_program_plane_noarm(plane, crtc_state, plane_state, color_plane);
-}
-
-static void
-skl_plane_update_arm(struct intel_plane *plane,
-const struct intel_crtc_state *crtc_state,
-const struct intel_plane_state *plane_state)
-{
-   int color_plane = 0;
-
-   if (plane_state->planar_linked_plane && !plane_state->planar_slave)
-   /* Program the UV plane on planar master */
-   color_plane = 1;
-
-   skl_program_plane_arm(plane, crtc_state, plane_state, color_plane);
-}
-
 static bool intel_format_is_p01x(u32 format)
 {
switch (format) {
-- 
2.32.0