Hi Ville,
On Thu, 2023-09-14 at 14:38 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Carve up tiling_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 | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 4c4626c84666..052f9d8b53d4 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -984,16 +984,21 @@ static bool intel_fbc_hw_tracking_covers_screen(const struct
> intel_plane_state *
> return effective_w <= max_w && effective_h <= max_h;
> }
>
> -static bool tiling_is_valid(const struct intel_plane_state *plane_state)
> +static bool i8xx_fbc_tiling_valid(const struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->hw.fb;
> +
> + return fb->modifier == I915_FORMAT_MOD_X_TILED;
> +}
> +
> +static bool skl_fbc_tiling_valid(const struct intel_plane_state *plane_state)
> {
> - struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev);
> const struct drm_framebuffer *fb = plane_state->hw.fb;
>
> switch (fb->modifier) {
> case DRM_FORMAT_MOD_LINEAR:
> case I915_FORMAT_MOD_Y_TILED:
> case I915_FORMAT_MOD_Yf_TILED:
> - return DISPLAY_VER(i915) >= 9;
> case I915_FORMAT_MOD_4_TILED:
> case I915_FORMAT_MOD_X_TILED:
> return true;
> @@ -1002,6 +1007,16 @@ static bool tiling_is_valid(const struct intel_plane_state *plane_state)
> }
> }
>
> +static bool tiling_is_valid(const struct intel_plane_state *plane_state)
> +{
> + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev);
> +
> + if (DISPLAY_VER(i915) >= 9)
> + return skl_fbc_tiling_valid(plane_state);
> + else
> + return i8xx_fbc_tiling_valid(plane_state);
I915_FORMAT_MOD_4_TILED is not checked for i8xx_fbc_tiling_valid() comparing to the original code.
Is that intentional?
With that checked,
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@xxxxxxxxx>
> +}
> +
> static void intel_fbc_update_state(struct intel_atomic_state *state,
> struct intel_crtc *crtc,
> struct intel_plane *plane)
