Re: [Intel-gfx] [PATCH] drm/i915: Fix ADL+ tiled plane stride when the POT stride is smaller than the original
I tried a bit if I can break something with ccs but it seemed everything work as expected with this fix. Reviewed-by: Juha-Pekka Heikkila On 4.12.2023 22.24, Ville Syrjala wrote: From: Ville Syrjälä plane_view_scanout_stride() currently assumes that we had to pad the mapping stride with dummy pages in order to align it. But that is not the case if the original fb stride exceeds the aligned stride used to populate the remapped view, which is calculated from the user specified framebuffer width rather than the user specified framebuffer stride. Ignore the original fb stride in this case and just stick to the POT aligned stride. Getting this wrong will cause the plane to fetch the wrong data, and can lead to fault errors if the page tables at the bogus location aren't even populated. TODO: figure out if this is OK for CCS, or if we should instead increase the width of the view to cover the entire user specified fb stride instead... Cc: Imre Deak Cc: Juha-Pekka Heikkila Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_fb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 33a693460420..ab634a4c86d1 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -1381,7 +1381,8 @@ plane_view_scanout_stride(const struct intel_framebuffer *fb, int color_plane, struct drm_i915_private *i915 = to_i915(fb->base.dev); unsigned int stride_tiles; - if (IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) >= 14) + if ((IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) >= 14) && + src_stride_tiles < dst_stride_tiles) stride_tiles = src_stride_tiles; else stride_tiles = dst_stride_tiles;
Re: [Intel-gfx] [PATCH] drm/i915: Fix ADL+ tiled plane stride when the POT stride is smaller than the original
On Mon, Dec 04, 2023 at 10:24:43PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > plane_view_scanout_stride() currently assumes that we had to pad the > mapping stride with dummy pages in order to align it. But that is not > the case if the original fb stride exceeds the aligned stride used > to populate the remapped view, which is calculated from the user > specified framebuffer width rather than the user specified framebuffer > stride. > > Ignore the original fb stride in this case and just stick to the POT > aligned stride. Getting this wrong will cause the plane to fetch the > wrong data, and can lead to fault errors if the page tables at the > bogus location aren't even populated. > > TODO: figure out if this is OK for CCS, or if we should instead increase > the width of the view to cover the entire user specified fb stride > instead... Yes, this is also needed since the CCS AUX surface can't be remapped in general (unless its stride is page size aligned -> main surface stride 256 tiles aligned). > Cc: Imre Deak > Cc: Juha-Pekka Heikkila > Signed-off-by: Ville Syrjälä Thanks for the fix, with the above CCS case also fixed as a follow-up or in this patch: Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/display/intel_fb.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c > b/drivers/gpu/drm/i915/display/intel_fb.c > index 33a693460420..ab634a4c86d1 100644 > --- a/drivers/gpu/drm/i915/display/intel_fb.c > +++ b/drivers/gpu/drm/i915/display/intel_fb.c > @@ -1381,7 +1381,8 @@ plane_view_scanout_stride(const struct > intel_framebuffer *fb, int color_plane, > struct drm_i915_private *i915 = to_i915(fb->base.dev); > unsigned int stride_tiles; > > - if (IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) >= 14) > + if ((IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) >= 14) && > + src_stride_tiles < dst_stride_tiles) > stride_tiles = src_stride_tiles; > else > stride_tiles = dst_stride_tiles; > -- > 2.41.0 >
[Intel-gfx] [PATCH] drm/i915: Fix ADL+ tiled plane stride when the POT stride is smaller than the original
From: Ville Syrjälä plane_view_scanout_stride() currently assumes that we had to pad the mapping stride with dummy pages in order to align it. But that is not the case if the original fb stride exceeds the aligned stride used to populate the remapped view, which is calculated from the user specified framebuffer width rather than the user specified framebuffer stride. Ignore the original fb stride in this case and just stick to the POT aligned stride. Getting this wrong will cause the plane to fetch the wrong data, and can lead to fault errors if the page tables at the bogus location aren't even populated. TODO: figure out if this is OK for CCS, or if we should instead increase the width of the view to cover the entire user specified fb stride instead... Cc: Imre Deak Cc: Juha-Pekka Heikkila Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_fb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c index 33a693460420..ab634a4c86d1 100644 --- a/drivers/gpu/drm/i915/display/intel_fb.c +++ b/drivers/gpu/drm/i915/display/intel_fb.c @@ -1381,7 +1381,8 @@ plane_view_scanout_stride(const struct intel_framebuffer *fb, int color_plane, struct drm_i915_private *i915 = to_i915(fb->base.dev); unsigned int stride_tiles; - if (IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) >= 14) + if ((IS_ALDERLAKE_P(i915) || DISPLAY_VER(i915) >= 14) && + src_stride_tiles < dst_stride_tiles) stride_tiles = src_stride_tiles; else stride_tiles = dst_stride_tiles; -- 2.41.0
