Hi Ville,
On Thu, 2023-09-14 at 14:38 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Carve up stride_is_valid() into per-platform variants to
> make it easier to see what limits are actually being imposed.
>
> TODO: maybe go for vfuncs later
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_fbc.c | 68 ++++++++++++++++++------
> 1 file changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 1b3358a0fbfb..4c4626c84666 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -848,6 +848,47 @@ void intel_fbc_cleanup(struct drm_i915_private *i915)
> }
> }
>
> +static bool i8xx_fbc_stride_is_valid(const struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->hw.fb;
> + unsigned int stride = intel_fbc_plane_stride(plane_state) *
> + fb->format->cpp[0];
> +
> + return stride == 4096 || stride == 8192;
> +}
> +
> +static bool i965_fbc_stride_is_valid(const struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->hw.fb;
> + unsigned int stride = intel_fbc_plane_stride(plane_state) *
> + fb->format->cpp[0];
> +
> + return stride >= 2048 && stride <= 16384;
> +}
> +
> +static bool g4x_fbc_stride_is_valid(const struct intel_plane_state *plane_state)
> +{
> + return true;
> +}
> +
> +static bool skl_fbc_stride_is_valid(const struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->hw.fb;
> + unsigned int stride = intel_fbc_plane_stride(plane_state) *
> + fb->format->cpp[0];
> +
> + /* Display WA #1105: skl,bxt,kbl,cfl,glk */
> + if (fb->modifier == DRM_FORMAT_MOD_LINEAR && stride & 511)
> + return false;
> +
> + return true;
> +}
> +
> +static bool icl_fbc_stride_is_valid(const struct intel_plane_state *plane_state)
> +{
> + return true;
> +}
> +
> static bool stride_is_valid(const struct intel_plane_state *plane_state)
> {
> struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev);
> @@ -859,23 +900,16 @@ static bool stride_is_valid(const struct intel_plane_state *plane_state)
> if (drm_WARN_ON_ONCE(&i915->drm, (stride & (64 - 1)) != 0))
> return false;
>
> - /* Below are the additional FBC restrictions. */
> - if (stride < 512)
> - return false;
Is this check not required anymore for ICL+ and G4x?
> -
> - if (DISPLAY_VER(i915) == 2 || DISPLAY_VER(i915) == 3)
> - return stride == 4096 || stride == 8192;
> -
> - if (DISPLAY_VER(i915) == 4 && !IS_G4X(i915) &&
> - (stride < 2048 || stride > 16384))
> - return false;
> -
> - /* Display WA #1105: skl,bxt,kbl,cfl,glk */
> - if ((DISPLAY_VER(i915) == 9 || IS_GEMINILAKE(i915)) &&
> - fb->modifier == DRM_FORMAT_MOD_LINEAR && stride & 511)
> - return false;
> -
> - return true;
> + if (DISPLAY_VER(i915) >= 11)
> + return icl_fbc_stride_is_valid(plane_state);
> + else if (DISPLAY_VER(i915) >= 9)
> + return skl_fbc_stride_is_valid(plane_state);
> + else if (DISPLAY_VER(i915) >= 5 || IS_G4X(i915))
> + return g4x_fbc_stride_is_valid(plane_state);
> + else if (DISPLAY_VER(i915) == 4)
> + return i965_fbc_stride_is_valid(plane_state);
> + else
> + return i8xx_fbc_stride_is_valid(plane_state);
Also I guess we could pass "stride" as parameter to these functions for clarity and simplify.
There as some IGT CI failures related to bad_stride tests.
BR
Vinod
> }
>
> static bool pixel_format_is_valid(const struct intel_plane_state *plane_state)
