On Tue, Nov 24, 2020 at 10:11:53PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> Currently crtc_state->uapi.plane_mask only tracks logically
> enabled planes on the uapi level. For bigjoiner purposes
> we want to do the same for the hw state. Let's follow the
> pattern established by active_planes & co. here.
> 
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_atomic_plane.c  |  3 +++
>  drivers/gpu/drm/i915/display/intel_display.c       | 13 +++++++++----
>  drivers/gpu/drm/i915/display/intel_display_types.h |  5 ++++-
>  3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c 
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 7e9f84b00859..b5e1ee99535c 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -312,10 +312,13 @@ int intel_plane_atomic_check_with_state(const struct 
> intel_crtc_state *old_crtc_
>       int ret;
>  
>       intel_plane_set_invisible(new_crtc_state, new_plane_state);
> +     new_crtc_state->enabled_planes &= ~BIT(plane->id);

Why not just add this a part of the intel_plane_set_invisible() function and 
may be rename that
to indicate invisible and disable?

Not a hard and fast requirement just a suggestion but in either case

Reviewed-by: Manasi Navare <manasi.d.nav...@intel.com>

Manasi

>  
>       if (!new_plane_state->hw.crtc && !old_plane_state->hw.crtc)
>               return 0;
>  
> +     new_crtc_state->enabled_planes |= BIT(plane->id);
> +
>       ret = plane->check_plane(new_crtc_state, new_plane_state);
>       if (ret)
>               return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 595183f7b60f..068892e4d2f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3551,7 +3551,7 @@ intel_set_plane_visible(struct intel_crtc_state 
> *crtc_state,
>               crtc_state->uapi.plane_mask &= ~drm_plane_mask(&plane->base);
>  }
>  
> -static void fixup_active_planes(struct intel_crtc_state *crtc_state)
> +static void fixup_plane_bitmasks(struct intel_crtc_state *crtc_state)
>  {
>       struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
>       struct drm_plane *plane;
> @@ -3561,11 +3561,14 @@ static void fixup_active_planes(struct 
> intel_crtc_state *crtc_state)
>        * have been used on the same (or wrong) pipe. plane_mask uses
>        * unique ids, hence we can use that to reconstruct active_planes.
>        */
> +     crtc_state->enabled_planes = 0;
>       crtc_state->active_planes = 0;
>  
>       drm_for_each_plane_mask(plane, &dev_priv->drm,
> -                             crtc_state->uapi.plane_mask)
> +                             crtc_state->uapi.plane_mask) {
> +             crtc_state->enabled_planes |= BIT(to_intel_plane(plane)->id);
>               crtc_state->active_planes |= BIT(to_intel_plane(plane)->id);
> +     }
>  }
>  
>  static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
> @@ -3583,7 +3586,7 @@ static void intel_plane_disable_noatomic(struct 
> intel_crtc *crtc,
>                   crtc->base.base.id, crtc->base.name);
>  
>       intel_set_plane_visible(crtc_state, plane_state, false);
> -     fixup_active_planes(crtc_state);
> +     fixup_plane_bitmasks(crtc_state);
>       crtc_state->data_rate[plane->id] = 0;
>       crtc_state->min_cdclk[plane->id] = 0;
>  
> @@ -12842,6 +12845,7 @@ static int icl_check_nv12_planes(struct 
> intel_crtc_state *crtc_state)
>  
>               plane_state->planar_linked_plane = NULL;
>               if (plane_state->planar_slave && !plane_state->uapi.visible) {
> +                     crtc_state->enabled_planes &= ~BIT(plane->id);
>                       crtc_state->active_planes &= ~BIT(plane->id);
>                       crtc_state->update_planes |= BIT(plane->id);
>               }
> @@ -12885,6 +12889,7 @@ static int icl_check_nv12_planes(struct 
> intel_crtc_state *crtc_state)
>  
>               linked_state->planar_slave = true;
>               linked_state->planar_linked_plane = plane;
> +             crtc_state->enabled_planes |= BIT(linked->id);
>               crtc_state->active_planes |= BIT(linked->id);
>               crtc_state->update_planes |= BIT(linked->id);
>               drm_dbg_kms(&dev_priv->drm, "Using %s as Y plane for %s\n",
> @@ -19165,7 +19170,7 @@ static void readout_plane_state(struct 
> drm_i915_private *dev_priv)
>               struct intel_crtc_state *crtc_state =
>                       to_intel_crtc_state(crtc->base.state);
>  
> -             fixup_active_planes(crtc_state);
> +             fixup_plane_bitmasks(crtc_state);
>       }
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index ce82d654d0f2..c93cf3ddebb6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1047,7 +1047,10 @@ struct intel_crtc_state {
>               u32 cgm_mode;
>       };
>  
> -     /* bitmask of visible planes (enum plane_id) */
> +     /* bitmask of logically enabled planes (enum plane_id) */
> +     u8 enabled_planes;
> +
> +     /* bitmask of actually visible planes (enum plane_id) */
>       u8 active_planes;
>       u8 nv12_planes;
>       u8 c8_planes;
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to