| Message ID | 20200924185113.30849-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add plane .{min, max}_width() and .max_height() vfuncs | expand |
On 9/24/20 11:51 AM, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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@linux.intel.com> LGTM.. Reviewed-by: Aditya Swarup <aditya.swarup@intel.com> > --- > 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; >
On 10/16/20 4:40 PM, Aditya Swarup wrote: > On 9/24/20 11:51 AM, Ville Syrjala wrote: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> 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@linux.intel.com> > LGTM.. > Reviewed-by: Aditya Swarup <aditya.swarup@intel.com> Hi Ville Are you going to push this patch to drm-tip or are you planning to rework this patch? This patch simplifies the max/min plane width plane assignment and fixes the NV12 aux surface bug and is good enough to push. Regards, Aditya Swarup >> --- >> 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Thu, Oct 22, 2020 at 05:17:00PM -0700, Aditya Swarup wrote: > On 10/16/20 4:40 PM, Aditya Swarup wrote: > > On 9/24/20 11:51 AM, Ville Syrjala wrote: > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> > >> 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@linux.intel.com> > > LGTM.. > > Reviewed-by: Aditya Swarup <aditya.swarup@intel.com> > Hi Ville > > Are you going to push this patch to drm-tip or are you planning to rework this patch? > This patch simplifies the max/min plane width plane assignment and fixes the NV12 aux surface bug > and is good enough to push. Didn't you have a related fix you wanted to get in first? > > Regards, > Aditya Swarup > >> --- > >> 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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
On 10/30/20 5:37 AM, Ville Syrjälä wrote: > On Thu, Oct 22, 2020 at 05:17:00PM -0700, Aditya Swarup wrote: >> On 10/16/20 4:40 PM, Aditya Swarup wrote: >>> On 9/24/20 11:51 AM, Ville Syrjala wrote: >>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>> >>>> 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@linux.intel.com> >>> LGTM.. >>> Reviewed-by: Aditya Swarup <aditya.swarup@intel.com> >> Hi Ville >> >> Are you going to push this patch to drm-tip or are you planning to rework this patch? >> This patch simplifies the max/min plane width plane assignment and fixes the NV12 aux surface bug >> and is good enough to push. > > Didn't you have a related fix you wanted to get in first? My patch is still unreviewed - https://patchwork.freedesktop.org/patch/394819/ and your patch fixes that issue as well in a cleaner way. So, I don't think it is worth the effort to push my patch and then rebase your patch. This patch should be sufficient. Aditya Swarup > >> >> Regards, >> Aditya Swarup >>>> --- >>>> 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@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >>> >
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;