On 9/24/20 11:51 AM, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Reduce this maintenance nightmare a bit by converting the plane > min/max width/height stuff into vfuncs. > > Now, if I could just think of a nice way to also use this for > intel_mode_valid_max_plane_size()... > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> LGTM.. Reviewed-by: Aditya Swarup <aditya.swarup@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_display.c | 194 +++++------------- > .../drm/i915/display/intel_display_types.h | 9 + > drivers/gpu/drm/i915/display/intel_sprite.c | 140 +++++++++++++ > 3 files changed, 196 insertions(+), 147 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 5a9d933e425a..a823d406f0ee 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3696,127 +3696,6 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > &to_intel_frontbuffer(fb)->bits); > } > > -static int skl_max_plane_width(const struct drm_framebuffer *fb, > - int color_plane, > - unsigned int rotation) > -{ > - int cpp = fb->format->cpp[color_plane]; > - > - switch (fb->modifier) { > - case DRM_FORMAT_MOD_LINEAR: > - case I915_FORMAT_MOD_X_TILED: > - /* > - * Validated limit is 4k, but has 5k should > - * work apart from the following features: > - * - Ytile (already limited to 4k) > - * - FP16 (already limited to 4k) > - * - render compression (already limited to 4k) > - * - KVMR sprite and cursor (don't care) > - * - horizontal panning (TODO verify this) > - * - pipe and plane scaling (TODO verify this) > - */ > - if (cpp == 8) > - return 4096; > - else > - return 5120; > - case I915_FORMAT_MOD_Y_TILED_CCS: > - case I915_FORMAT_MOD_Yf_TILED_CCS: > - case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS: > - /* FIXME AUX plane? */ > - case I915_FORMAT_MOD_Y_TILED: > - case I915_FORMAT_MOD_Yf_TILED: > - if (cpp == 8) > - return 2048; > - else > - return 4096; > - default: > - MISSING_CASE(fb->modifier); > - return 2048; > - } > -} > - > -static int glk_max_plane_width(const struct drm_framebuffer *fb, > - int color_plane, > - unsigned int rotation) > -{ > - int cpp = fb->format->cpp[color_plane]; > - > - switch (fb->modifier) { > - case DRM_FORMAT_MOD_LINEAR: > - case I915_FORMAT_MOD_X_TILED: > - if (cpp == 8) > - return 4096; > - else > - return 5120; > - case I915_FORMAT_MOD_Y_TILED_CCS: > - case I915_FORMAT_MOD_Yf_TILED_CCS: > - /* FIXME AUX plane? */ > - case I915_FORMAT_MOD_Y_TILED: > - case I915_FORMAT_MOD_Yf_TILED: > - if (cpp == 8) > - return 2048; > - else > - return 5120; > - default: > - MISSING_CASE(fb->modifier); > - return 2048; > - } > -} > - > -static int icl_min_plane_width(const struct drm_framebuffer *fb) > -{ > - /* Wa_14011264657, Wa_14011050563: gen11+ */ > - switch (fb->format->format) { > - case DRM_FORMAT_C8: > - return 18; > - case DRM_FORMAT_RGB565: > - return 10; > - case DRM_FORMAT_XRGB8888: > - case DRM_FORMAT_XBGR8888: > - case DRM_FORMAT_ARGB8888: > - case DRM_FORMAT_ABGR8888: > - case DRM_FORMAT_XRGB2101010: > - case DRM_FORMAT_XBGR2101010: > - case DRM_FORMAT_ARGB2101010: > - case DRM_FORMAT_ABGR2101010: > - case DRM_FORMAT_XVYU2101010: > - case DRM_FORMAT_Y212: > - case DRM_FORMAT_Y216: > - return 6; > - case DRM_FORMAT_NV12: > - return 20; > - case DRM_FORMAT_P010: > - case DRM_FORMAT_P012: > - case DRM_FORMAT_P016: > - return 12; > - case DRM_FORMAT_XRGB16161616F: > - case DRM_FORMAT_XBGR16161616F: > - case DRM_FORMAT_ARGB16161616F: > - case DRM_FORMAT_ABGR16161616F: > - case DRM_FORMAT_XVYU12_16161616: > - case DRM_FORMAT_XVYU16161616: > - return 4; > - default: > - return 1; > - } > -} > - > -static int icl_max_plane_width(const struct drm_framebuffer *fb, > - int color_plane, > - unsigned int rotation) > -{ > - return 5120; > -} > - > -static int skl_max_plane_height(void) > -{ > - return 4096; > -} > - > -static int icl_max_plane_height(void) > -{ > - return 4320; > -} > > static bool > skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state, > @@ -3874,35 +3753,55 @@ intel_plane_fence_y_offset(const struct intel_plane_state *plane_state) > return y; > } > > +static int intel_plane_min_width(struct intel_plane *plane, > + const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + if (plane->min_width) > + return plane->min_width(fb, color_plane, rotation); > + else > + return 1; > +} > + > +static int intel_plane_max_width(struct intel_plane *plane, > + const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + if (plane->max_width) > + return plane->max_width(fb, color_plane, rotation); > + else > + return INT_MAX; > +} > + > +static int intel_plane_max_height(struct intel_plane *plane, > + const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + if (plane->max_height) > + return plane->max_height(fb, color_plane, rotation); > + else > + return INT_MAX; > +} > + > static int skl_check_main_surface(struct intel_plane_state *plane_state) > { > - struct drm_i915_private *dev_priv = to_i915(plane_state->uapi.plane->dev); > + struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > const struct drm_framebuffer *fb = plane_state->hw.fb; > unsigned int rotation = plane_state->hw.rotation; > int x = plane_state->uapi.src.x1 >> 16; > int y = plane_state->uapi.src.y1 >> 16; > int w = drm_rect_width(&plane_state->uapi.src) >> 16; > int h = drm_rect_height(&plane_state->uapi.src) >> 16; > - int max_width, min_width, max_height; > - u32 alignment, offset; > + int min_width = intel_plane_min_width(plane, fb, 0, rotation); > + int max_width = intel_plane_max_width(plane, fb, 0, rotation); > + int max_height = intel_plane_max_height(plane, fb, 0, rotation); > int aux_plane = intel_main_to_aux_plane(fb, 0); > u32 aux_offset = plane_state->color_plane[aux_plane].offset; > - > - if (INTEL_GEN(dev_priv) >= 11) { > - max_width = icl_max_plane_width(fb, 0, rotation); > - min_width = icl_min_plane_width(fb); > - } else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) { > - max_width = glk_max_plane_width(fb, 0, rotation); > - min_width = 1; > - } else { > - max_width = skl_max_plane_width(fb, 0, rotation); > - min_width = 1; > - } > - > - if (INTEL_GEN(dev_priv) >= 11) > - max_height = icl_max_plane_height(); > - else > - max_height = skl_max_plane_height(); > + u32 alignment, offset; > > if (w > max_width || w < min_width || h > max_height) { > drm_dbg_kms(&dev_priv->drm, > @@ -3985,22 +3884,19 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state) > > static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state) > { > - struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); > + struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); > + struct drm_i915_private *i915 = to_i915(plane->base.dev); > const struct drm_framebuffer *fb = plane_state->hw.fb; > unsigned int rotation = plane_state->hw.rotation; > int uv_plane = 1; > - int max_width = skl_max_plane_width(fb, uv_plane, rotation); > - int max_height = 4096; > + int max_width = intel_plane_max_width(plane, fb, uv_plane, rotation); > + int max_height = intel_plane_max_height(plane, fb, uv_plane, rotation); > int x = plane_state->uapi.src.x1 >> 17; > int y = plane_state->uapi.src.y1 >> 17; > int w = drm_rect_width(&plane_state->uapi.src) >> 17; > int h = drm_rect_height(&plane_state->uapi.src) >> 17; > u32 offset; > > - intel_add_fb_offsets(&x, &y, plane_state, uv_plane); > - offset = intel_plane_compute_aligned_offset(&x, &y, > - plane_state, uv_plane); > - > /* FIXME not quite sure how/if these apply to the chroma plane */ > if (w > max_width || h > max_height) { > drm_dbg_kms(&i915->drm, > @@ -4009,6 +3905,10 @@ static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state) > return -EINVAL; > } > > + intel_add_fb_offsets(&x, &y, plane_state, uv_plane); > + offset = intel_plane_compute_aligned_offset(&x, &y, > + plane_state, uv_plane); > + > if (is_ccs_modifier(fb->modifier)) { > int ccs_plane = main_to_ccs_plane(fb, uv_plane); > int aux_offset = plane_state->color_plane[ccs_plane].offset; > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index 3d4bf9b6a0a2..a16120508318 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1170,6 +1170,15 @@ struct intel_plane { > * the intel_plane_state structure and accessed via plane_state. > */ > > + int (*min_width)(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation); > + int (*max_width)(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation); > + int (*max_height)(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation); > unsigned int (*max_stride)(struct intel_plane *plane, > u32 pixel_format, u64 modifier, > unsigned int rotation); > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c > index 63040cb0d4e1..d682ab1c0f70 100644 > --- a/drivers/gpu/drm/i915/display/intel_sprite.c > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c > @@ -393,6 +393,134 @@ static int skl_plane_min_cdclk(const struct intel_crtc_state *crtc_state, > return DIV_ROUND_UP(pixel_rate * num, den); > } > > +static int skl_plane_max_width(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + int cpp = fb->format->cpp[color_plane]; > + > + switch (fb->modifier) { > + case DRM_FORMAT_MOD_LINEAR: > + case I915_FORMAT_MOD_X_TILED: > + /* > + * Validated limit is 4k, but has 5k should > + * work apart from the following features: > + * - Ytile (already limited to 4k) > + * - FP16 (already limited to 4k) > + * - render compression (already limited to 4k) > + * - KVMR sprite and cursor (don't care) > + * - horizontal panning (TODO verify this) > + * - pipe and plane scaling (TODO verify this) > + */ > + if (cpp == 8) > + return 4096; > + else > + return 5120; > + case I915_FORMAT_MOD_Y_TILED_CCS: > + case I915_FORMAT_MOD_Yf_TILED_CCS: > + case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS: > + /* FIXME AUX plane? */ > + case I915_FORMAT_MOD_Y_TILED: > + case I915_FORMAT_MOD_Yf_TILED: > + if (cpp == 8) > + return 2048; > + else > + return 4096; > + default: > + MISSING_CASE(fb->modifier); > + return 2048; > + } > +} > + > +static int glk_plane_max_width(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + int cpp = fb->format->cpp[color_plane]; > + > + switch (fb->modifier) { > + case DRM_FORMAT_MOD_LINEAR: > + case I915_FORMAT_MOD_X_TILED: > + if (cpp == 8) > + return 4096; > + else > + return 5120; > + case I915_FORMAT_MOD_Y_TILED_CCS: > + case I915_FORMAT_MOD_Yf_TILED_CCS: > + /* FIXME AUX plane? */ > + case I915_FORMAT_MOD_Y_TILED: > + case I915_FORMAT_MOD_Yf_TILED: > + if (cpp == 8) > + return 2048; > + else > + return 5120; > + default: > + MISSING_CASE(fb->modifier); > + return 2048; > + } > +} > + > +static int icl_plane_min_width(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + /* Wa_14011264657, Wa_14011050563: gen11+ */ > + switch (fb->format->format) { > + case DRM_FORMAT_C8: > + return 18; > + case DRM_FORMAT_RGB565: > + return 10; > + case DRM_FORMAT_XRGB8888: > + case DRM_FORMAT_XBGR8888: > + case DRM_FORMAT_ARGB8888: > + case DRM_FORMAT_ABGR8888: > + case DRM_FORMAT_XRGB2101010: > + case DRM_FORMAT_XBGR2101010: > + case DRM_FORMAT_ARGB2101010: > + case DRM_FORMAT_ABGR2101010: > + case DRM_FORMAT_XVYU2101010: > + case DRM_FORMAT_Y212: > + case DRM_FORMAT_Y216: > + return 6; > + case DRM_FORMAT_NV12: > + return 20; > + case DRM_FORMAT_P010: > + case DRM_FORMAT_P012: > + case DRM_FORMAT_P016: > + return 12; > + case DRM_FORMAT_XRGB16161616F: > + case DRM_FORMAT_XBGR16161616F: > + case DRM_FORMAT_ARGB16161616F: > + case DRM_FORMAT_ABGR16161616F: > + case DRM_FORMAT_XVYU12_16161616: > + case DRM_FORMAT_XVYU16161616: > + return 4; > + default: > + return 1; > + } > +} > + > +static int icl_plane_max_width(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + return 5120; > +} > + > +static int skl_plane_max_height(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + return 4096; > +} > + > +static int icl_plane_max_height(const struct drm_framebuffer *fb, > + int color_plane, > + unsigned int rotation) > +{ > + return 4320; > +} > + > static unsigned int > skl_plane_max_stride(struct intel_plane *plane, > u32 pixel_format, u64 modifier, > @@ -3083,6 +3211,18 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv, > fbc->possible_framebuffer_bits |= plane->frontbuffer_bit; > } > > + if (INTEL_GEN(dev_priv) >= 11) { > + plane->min_width = icl_plane_min_width; > + plane->max_width = icl_plane_max_width; > + plane->max_height = icl_plane_max_height; > + } else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) { > + plane->max_width = glk_plane_max_width; > + plane->max_height = skl_plane_max_height; > + } else { > + plane->max_width = skl_plane_max_width; > + plane->max_height = skl_plane_max_height; > + } > + > plane->max_stride = skl_plane_max_stride; > plane->update_plane = skl_update_plane; > plane->disable_plane = skl_disable_plane; > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx