On Sat, Jul 19, 2025 at 09:23:04PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 18, 2025 at 11:31:21AM +0200, Jocelyn Falempe wrote:
> > drm_panic draws in linear framebuffer, so it's easier to re-use the
> > current framebuffer, and disable tiling in the panic handler, to show
> > the panic screen.
> > This assumes that the alignment restriction is always smaller in
> > linear than in tiled.
> > It also assumes that the linear framebuffer size is always smaller
> > than the tiled.
> >
> > Signed-off-by: Jocelyn Falempe <jfale...@redhat.com>
> > ---
> >
> > v7:
> > * Reword commit message about alignment/size when disabling tiling (Ville
> > Syrjälä)
> >
> > drivers/gpu/drm/i915/display/i9xx_plane.c | 23 +++++++++++++++++++
> > .../drm/i915/display/intel_display_types.h | 2 ++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c
> > b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > index 8f15333a4b07..0807fae12450 100644
> > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c
> > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
> > @@ -905,6 +905,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs =
> > {
> > .format_mod_supported_async = intel_plane_format_mod_supported_async,
> > };
> >
> > +static void i9xx_disable_tiling(struct intel_plane *plane)
> > +{
> > + struct intel_display *display = to_intel_display(plane);
> > + enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
> > + u32 dspcntr;
> > + u32 reg;
> > +
> > + dspcntr = intel_de_read_fw(display, DSPCNTR(display, i9xx_plane));
> > + dspcntr &= ~DISP_TILED;
> > + intel_de_write_fw(display, DSPCNTR(display, i9xx_plane), dspcntr);
> > +
> > + if (DISPLAY_VER(display) >= 4) {
> > + reg = intel_de_read_fw(display, DSPSURF(display, i9xx_plane));
> > + intel_de_write_fw(display, DSPSURF(display, i9xx_plane), reg);
> > +
> > + } else {
> > + reg = intel_de_read_fw(display, DSPADDR(display, i9xx_plane));
> > + intel_de_write_fw(display, DSPADDR(display, i9xx_plane), reg);
> > + }
> > +}
>
> I thought I already shot this down before, but apparently this
> got merged now :(
>
> Just to reiterate why we don't want these 'disable tiling' hacks:
> - different tiling formats have different stride/alignment/watermark
> requirements so one can't safely change from one tiling to another
> - this completely fails to account for the TILEOFF vs. LINOFF stuff
Oh yeah, and rotation support of course is one really big difference
between different tiling formats.
> - etc.
>
> So IMO these hacks must be removed and instead the code must learn how
> to propetly write the tiled data. igt has all the code for that btw
> (twice over IIRC) so shouldn't be that hard.
>
> I suppose the only hack we need to keep is to disable compression,
> mainly because (IIRC) on flat CCS systems the CPU doesn't have access
> to the AUX data to clear it manually.
>
> I also wonder if there are actual igts for this? I think what is needed
> is a test that sets random things (different panning, rotation, pixel
> foramts, etc.) and triggers the dumper. Not quite sure how the test
> could validate that the output is correct though. CRCs might be a bit
> tricky since you need an identical reference image.
>
> /me off to summer vacation. Good luck
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
