Re: [Intel-gfx] [PATCH 04/10] drm/i915/gen9: Cleanup skl_pipe_wm_active_state

2016-10-11 Thread Paulo Zanoni 
Em Sex, 2016-10-07 às 20:11 -0400, Lyude escreveu:
> This function is a wreck, let's help it get it's life back together
> and
> cleanup all of the copy pasta here.

s/it's/its/

Idea for your next patch series: rename skl_pipe_wm_active_state()'s
"i" parameter to something more meaningful.

Reviewed-by: Paulo Zanoni 

> 
> (adding Maarten's reviewed-by since this is just a split-up version
> of one
> of the previous patches)
> 
> Signed-off-by: Lyude 
> Reviewed-by: Maarten Lankhorst 
> Cc: Ville Syrjälä 
> Cc: Paulo Zanoni 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 52 +++--
> 
>  1 file changed, 14 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 4c2ebcd..5dbaf12 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4267,46 +4267,22 @@ static void ilk_optimize_watermarks(struct
> intel_crtc_state *cstate)
>  static void skl_pipe_wm_active_state(uint32_t val,
>    struct skl_pipe_wm *active,
>    bool is_transwm,
> -  bool is_cursor,
>    int i,
>    int level)
>  {
> + struct skl_plane_wm *plane_wm = >planes[i];
>   bool is_enabled = (val & PLANE_WM_EN) != 0;
>  
>   if (!is_transwm) {
> - if (!is_cursor) {
> - active->planes[i].wm[level].plane_en =
> is_enabled;
> - active->planes[i].wm[level].plane_res_b =
> - val & PLANE_WM_BLOCKS_MASK;
> - active->planes[i].wm[level].plane_res_l =
> - (val >> PLANE_WM_LINES_SHIFT) &
> - PLANE_WM_LINES_MASK;
> - } else {
> - active-
> >planes[PLANE_CURSOR].wm[level].plane_en =
> - is_enabled;
> - active-
> >planes[PLANE_CURSOR].wm[level].plane_res_b =
> - val & PLANE_WM_BLOCKS_MASK;
> - active-
> >planes[PLANE_CURSOR].wm[level].plane_res_l =
> - (val >> PLANE_WM_LINES_SHIFT) &
> - PLANE_WM_LINES_MASK;
> - }
> + plane_wm->wm[level].plane_en = is_enabled;
> + plane_wm->wm[level].plane_res_b = val &
> PLANE_WM_BLOCKS_MASK;
> + plane_wm->wm[level].plane_res_l =
> + (val >> PLANE_WM_LINES_SHIFT) &
> PLANE_WM_LINES_MASK;
>   } else {
> - if (!is_cursor) {
> - active->planes[i].trans_wm.plane_en =
> is_enabled;
> - active->planes[i].trans_wm.plane_res_b =
> - val & PLANE_WM_BLOCKS_MASK;
> - active->planes[i].trans_wm.plane_res_l =
> - (val >> PLANE_WM_LINES_SHIFT) &
> - PLANE_WM_LINES_MASK;
> - } else {
> - active-
> >planes[PLANE_CURSOR].trans_wm.plane_en =
> - is_enabled;
> - active-
> >planes[PLANE_CURSOR].trans_wm.plane_res_b =
> - val & PLANE_WM_BLOCKS_MASK;
> - active-
> >planes[PLANE_CURSOR].trans_wm.plane_res_l =
> - (val >> PLANE_WM_LINES_SHIFT) &
> - PLANE_WM_LINES_MASK;
> - }
> + plane_wm->trans_wm.plane_en = is_enabled;
> + plane_wm->trans_wm.plane_res_b = val &
> PLANE_WM_BLOCKS_MASK;
> + plane_wm->trans_wm.plane_res_l =
> + (val >> PLANE_WM_LINES_SHIFT) &
> PLANE_WM_LINES_MASK;
>   }
>  }
>  
> @@ -4345,20 +4321,20 @@ static void skl_pipe_wm_get_hw_state(struct
> drm_crtc *crtc)
>   for (level = 0; level <= max_level; level++) {
>   for (i = 0; i < intel_num_planes(intel_crtc); i++) {
>   temp = hw->plane[pipe][i][level];
> - skl_pipe_wm_active_state(temp, active,
> false,
> - false, i, level);
> + skl_pipe_wm_active_state(temp, active,
> false, i, level);
>   }
>   temp = hw->plane[pipe][PLANE_CURSOR][level];
> - skl_pipe_wm_active_state(temp, active, false, true,
> i, level);
> + skl_pipe_wm_active_state(temp, active, false,
> PLANE_CURSOR,
> +  level);
>   }
>  
>   for (i = 0; i < intel_num_planes(intel_crtc); i++) {
>   temp = hw->plane_trans[pipe][i];
> - skl_pipe_wm_active_state(temp, active, true, false,
> i, 0);
> + skl_pipe_wm_active_state(temp,

[Intel-gfx] [PATCH 04/10] drm/i915/gen9: Cleanup skl_pipe_wm_active_state

2016-10-07 Thread Lyude 
This function is a wreck, let's help it get it's life back together and
cleanup all of the copy pasta here.

(adding Maarten's reviewed-by since this is just a split-up version of one
of the previous patches)

Signed-off-by: Lyude 
Reviewed-by: Maarten Lankhorst 
Cc: Ville Syrjälä 
Cc: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_pm.c | 52 +++--
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c2ebcd..5dbaf12 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4267,46 +4267,22 @@ static void ilk_optimize_watermarks(struct 
intel_crtc_state *cstate)
 static void skl_pipe_wm_active_state(uint32_t val,
 struct skl_pipe_wm *active,
 bool is_transwm,
-bool is_cursor,
 int i,
 int level)
 {
+   struct skl_plane_wm *plane_wm = >planes[i];
bool is_enabled = (val & PLANE_WM_EN) != 0;
 
if (!is_transwm) {
-   if (!is_cursor) {
-   active->planes[i].wm[level].plane_en = is_enabled;
-   active->planes[i].wm[level].plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[i].wm[level].plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   } else {
-   active->planes[PLANE_CURSOR].wm[level].plane_en =
-   is_enabled;
-   active->planes[PLANE_CURSOR].wm[level].plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[PLANE_CURSOR].wm[level].plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   }
+   plane_wm->wm[level].plane_en = is_enabled;
+   plane_wm->wm[level].plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+   plane_wm->wm[level].plane_res_l =
+   (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
} else {
-   if (!is_cursor) {
-   active->planes[i].trans_wm.plane_en = is_enabled;
-   active->planes[i].trans_wm.plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[i].trans_wm.plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   } else {
-   active->planes[PLANE_CURSOR].trans_wm.plane_en =
-   is_enabled;
-   active->planes[PLANE_CURSOR].trans_wm.plane_res_b =
-   val & PLANE_WM_BLOCKS_MASK;
-   active->planes[PLANE_CURSOR].trans_wm.plane_res_l =
-   (val >> PLANE_WM_LINES_SHIFT) &
-   PLANE_WM_LINES_MASK;
-   }
+   plane_wm->trans_wm.plane_en = is_enabled;
+   plane_wm->trans_wm.plane_res_b = val & PLANE_WM_BLOCKS_MASK;
+   plane_wm->trans_wm.plane_res_l =
+   (val >> PLANE_WM_LINES_SHIFT) & PLANE_WM_LINES_MASK;
}
 }
 
@@ -4345,20 +4321,20 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc 
*crtc)
for (level = 0; level <= max_level; level++) {
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane[pipe][i][level];
-   skl_pipe_wm_active_state(temp, active, false,
-   false, i, level);
+   skl_pipe_wm_active_state(temp, active, false, i, level);
}
temp = hw->plane[pipe][PLANE_CURSOR][level];
-   skl_pipe_wm_active_state(temp, active, false, true, i, level);
+   skl_pipe_wm_active_state(temp, active, false, PLANE_CURSOR,
+level);
}
 
for (i = 0; i < intel_num_planes(intel_crtc); i++) {
temp = hw->plane_trans[pipe][i];
-   skl_pipe_wm_active_state(temp, active, true, false, i, 0);
+   skl_pipe_wm_active_state(temp, active, true, i, 0);
}
 
temp = hw->plane_trans[pipe][PLANE_CURSOR];
-   skl_pipe_wm_active_state(temp, active, true, true, i, 0);
+   skl_pipe_wm_active_state(temp, active, true, PLANE_CURSOR, 0);
 
intel_crtc->wm.active.skl = *active;
 }
-- 
2.7.4

___