[Intel-gfx] [PATCH 05/15] drm/i915: Add helpers to select correct ccs/aux planes
Imre Deak
imre.deak at intel.com
Fri Dec 20 14:23:56 UTC 2019
On Fri, Dec 20, 2019 at 04:03:20PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 20, 2019 at 02:26:07AM +0200, Imre Deak wrote:
> > On Thu, Dec 19, 2019 at 01:04:33PM -0800, Matt Roper wrote:
> > > On Wed, Dec 18, 2019 at 06:10:55PM +0200, Imre Deak wrote:
> > > > Using helpers instead of open coding this to select a CCS plane for a
> > > > main plane makes the code cleaner and less error-prone when the location
> > > > of CCS plane can be different based on the format (packed vs. YUV
> > > > semiplanar). The same applies to selecting an AUX plane which can be a
> > > > UV plane (for an uncompressed YUV semiplanar format), or a CCS plane.
> > > >
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > >
> > > Looking at this makes me wonder if some of the aux stuff that we're
> > > doing for YUV in skl_check_main_surface is actually necessary for gen11+
> > > now that we have separate planes rather than an AUX surface in the same
> > > plane.
> >
> > I also wondered if programming the UV surface's offset in the Y plane's
> > AUX_DIST register is necessary or not on GEN11+, however that's what we
> > do atm.
>
> Shouldn't be needed. But I presume you're changing that anyway for media
> compression since you need aux for both Y and UV?
Right, CCS planes (both for RGB and YUV) will need to be programmed as
an AUX plane of their main surface (and for that the
main_to_aux_plane() helper came handy since the code is shared since
GEN9).
What I was pondering after Matt's comment is that for GEN11 we could
stop programming the UV AUX plane in skl_program_plane() which means the
UV coords need not match the main plane coords, what we require now even
on GEN11 (otoh for CCS AUX we need coordinates to match there too).
But this is unrelated to the change in this patch, so we can ignore that
for now.
> > >
> > > But none of the logic should be impacted by your changes here so,
> > >
> > > Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> > >
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display.c | 63 ++++++++++++++++----
> > > > 1 file changed, 50 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 4b8b44c39724..6bda397ae677 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -1933,6 +1933,40 @@ static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv)
> > > > return IS_GEN(dev_priv, 2) ? 2048 : 4096;
> > > > }
> > > >
> > > > +static bool is_ccs_plane(const struct drm_framebuffer *fb, int plane)
> > > > +{
> > > > + if (!is_ccs_modifier(fb->modifier))
> > > > + return false;
> > > > +
> > > > + return plane >= fb->format->num_planes / 2;
> > > > +}
> > > > +
> > > > +static bool is_aux_plane(const struct drm_framebuffer *fb, int plane)
> > > > +{
> > > > + if (is_ccs_modifier(fb->modifier))
> > > > + return is_ccs_plane(fb, plane);
> > > > +
> > > > + return plane == 1;
> > > > +}
> > > > +
> > > > +static int main_to_ccs_plane(const struct drm_framebuffer *fb, int main_plane)
> > > > +{
> > > > + WARN_ON(!is_ccs_modifier(fb->modifier) ||
> > > > + (main_plane && main_plane >= fb->format->num_planes / 2));
> > > > +
> > > > + return fb->format->num_planes / 2 + main_plane;
> > > > +}
> > > > +
> > > > +/* Return either the main plane's CCS or - if not a CCS FB - UV plane */
> > > > +static int
> > > > +intel_main_to_aux_plane(const struct drm_framebuffer *fb, int main_plane)
> > > > +{
> > > > + if (is_ccs_modifier(fb->modifier))
> > > > + return main_to_ccs_plane(fb, main_plane);
> > > > +
> > > > + return 1;
> > > > +}
> > > > +
> > > > static unsigned int
> > > > intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
> > > > {
> > > > @@ -1948,7 +1982,7 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
> > > > else
> > > > return 512;
> > > > case I915_FORMAT_MOD_Y_TILED_CCS:
> > > > - if (color_plane == 1)
> > > > + if (is_ccs_plane(fb, color_plane))
> > > > return 128;
> > > > /* fall through */
> > > > case I915_FORMAT_MOD_Y_TILED:
> > > > @@ -1957,7 +1991,7 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
> > > > else
> > > > return 512;
> > > > case I915_FORMAT_MOD_Yf_TILED_CCS:
> > > > - if (color_plane == 1)
> > > > + if (is_ccs_plane(fb, color_plane))
> > > > return 128;
> > > > /* fall through */
> > > > case I915_FORMAT_MOD_Yf_TILED:
> > > > @@ -2074,7 +2108,7 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
> > > > struct drm_i915_private *dev_priv = to_i915(fb->dev);
> > > >
> > > > /* AUX_DIST needs only 4K alignment */
> > > > - if (color_plane == 1)
> > > > + if (is_aux_plane(fb, color_plane))
> > > > return 4096;
> > > >
> > > > switch (fb->modifier) {
> > > > @@ -3457,10 +3491,11 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
> > > > const struct drm_framebuffer *fb = plane_state->hw.fb;
> > > > int hsub = fb->format->hsub;
> > > > int vsub = fb->format->vsub;
> > > > - int aux_x = plane_state->color_plane[1].x;
> > > > - int aux_y = plane_state->color_plane[1].y;
> > > > - u32 aux_offset = plane_state->color_plane[1].offset;
> > > > - u32 alignment = intel_surf_alignment(fb, 1);
> > > > + int ccs_plane = main_to_ccs_plane(fb, 0);
> > > > + int aux_x = plane_state->color_plane[ccs_plane].x;
> > > > + int aux_y = plane_state->color_plane[ccs_plane].y;
> > > > + u32 aux_offset = plane_state->color_plane[ccs_plane].offset;
> > > > + u32 alignment = intel_surf_alignment(fb, ccs_plane);
> > > >
> > > > while (aux_offset >= main_offset && aux_y <= main_y) {
> > > > int x, y;
> > > > @@ -3473,7 +3508,7 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
> > > >
> > > > x = aux_x / hsub;
> > > > y = aux_y / vsub;
> > > > - aux_offset = intel_plane_adjust_aligned_offset(&x, &y, plane_state, 1,
> > > > + aux_offset = intel_plane_adjust_aligned_offset(&x, &y, plane_state, ccs_plane,
> > > > aux_offset, aux_offset - alignment);
> > > > aux_x = x * hsub + aux_x % hsub;
> > > > aux_y = y * vsub + aux_y % vsub;
> > > > @@ -3482,9 +3517,9 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
> > > > if (aux_x != main_x || aux_y != main_y)
> > > > return false;
> > > >
> > > > - plane_state->color_plane[1].offset = aux_offset;
> > > > - plane_state->color_plane[1].x = aux_x;
> > > > - plane_state->color_plane[1].y = aux_y;
> > > > + plane_state->color_plane[ccs_plane].offset = aux_offset;
> > > > + plane_state->color_plane[ccs_plane].x = aux_x;
> > > > + plane_state->color_plane[ccs_plane].y = aux_y;
> > > >
> > > > return true;
> > > > }
> > > > @@ -3500,7 +3535,8 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > > > int h = drm_rect_height(&plane_state->uapi.src) >> 16;
> > > > int max_width;
> > > > int max_height;
> > > > - u32 alignment, offset, aux_offset = plane_state->color_plane[1].offset;
> > > > + int aux_plane = intel_main_to_aux_plane(fb, 0);
> > > > + u32 alignment, offset, aux_offset = plane_state->color_plane[aux_plane].offset;
> > > >
> > > > if (INTEL_GEN(dev_priv) >= 11)
> > > > max_width = icl_max_plane_width(fb, 0, rotation);
> > > > @@ -3566,7 +3602,8 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > > > offset, offset - alignment);
> > > > }
> > > >
> > > > - if (x != plane_state->color_plane[1].x || y != plane_state->color_plane[1].y) {
> > > > + if (x != plane_state->color_plane[aux_plane].x ||
> > > > + y != plane_state->color_plane[aux_plane].y) {
> > > > DRM_DEBUG_KMS("Unable to find suitable display surface offset due to CCS\n");
> > > > return -EINVAL;
> > > > }
> > > > --
> > > > 2.22.0
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Matt Roper
> > > Graphics Software Engineer
> > > VTT-OSGC Platform Enablement
> > > Intel Corporation
> > > (916) 356-2795
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
More information about the Intel-gfx
mailing list