| Message ID | 20250401125818.333033-3-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915: Add drm_panic support | expand |
On Tue, Apr 01, 2025 at 02:51:08PM +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. > > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > --- > 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 5e8344fdfc28..9c93d5ac7129 100644 > --- a/drivers/gpu/drm/i915/display/i9xx_plane.c > +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c > @@ -908,6 +908,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs = { > .format_mod_supported = i8xx_plane_format_mod_supported, > }; > > +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); This fails to account all the different alignment/etc. restrictions between linear vs. tiled. I don't think we want hacks like this. > + > + 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); > + } > +} > + > struct intel_plane * > intel_primary_plane_create(struct intel_display *display, enum pipe pipe) > { > @@ -1050,6 +1071,8 @@ intel_primary_plane_create(struct intel_display *display, enum pipe pipe) > } > } > > + plane->disable_tiling = i9xx_disable_tiling; > + > modifiers = intel_fb_plane_get_modifiers(display, INTEL_PLANE_CAP_TILING_X); > > if (DISPLAY_VER(display) >= 5 || display->platform.g4x) > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index 367b53a9eae2..62d0785c9edf 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1512,6 +1512,8 @@ struct intel_plane { > bool async_flip); > void (*enable_flip_done)(struct intel_plane *plane); > void (*disable_flip_done)(struct intel_plane *plane); > + /* For drm_panic */ > + void (*disable_tiling)(struct intel_plane *plane); > }; > > #define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base) > -- > 2.49.0
On 01/04/2025 19:38, Ville Syrjälä wrote: > On Tue, Apr 01, 2025 at 02:51:08PM +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. >> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> --- >> 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 5e8344fdfc28..9c93d5ac7129 100644 >> --- a/drivers/gpu/drm/i915/display/i9xx_plane.c >> +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c >> @@ -908,6 +908,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs = { >> .format_mod_supported = i8xx_plane_format_mod_supported, >> }; >> >> +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); > > This fails to account all the different alignment/etc. restrictions > between linear vs. tiled. I don't think we want hacks like this. Thanks for taking a look. I assumed that linear have always less alignment restrictions than tiled. I also assumed that the framebuffer size in linear is smaller than tiled (as we keep the same pixel format). So going from tiled to linear should be safe, the other way is not. It's done this way in amdgpu [1], but I agree it might be different on Intel hardware The alternative is to draw with tiling, which is what I have done for Y-tile and 4-tile format, so it's also a possibility, but more complex to maintain. [1] https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/gpu/drm/amd/display/dc/core/dc_surface.c#L298 Best regards,
diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c index 5e8344fdfc28..9c93d5ac7129 100644 --- a/drivers/gpu/drm/i915/display/i9xx_plane.c +++ b/drivers/gpu/drm/i915/display/i9xx_plane.c @@ -908,6 +908,27 @@ static const struct drm_plane_funcs i8xx_plane_funcs = { .format_mod_supported = i8xx_plane_format_mod_supported, }; +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); + } +} + struct intel_plane * intel_primary_plane_create(struct intel_display *display, enum pipe pipe) { @@ -1050,6 +1071,8 @@ intel_primary_plane_create(struct intel_display *display, enum pipe pipe) } } + plane->disable_tiling = i9xx_disable_tiling; + modifiers = intel_fb_plane_get_modifiers(display, INTEL_PLANE_CAP_TILING_X); if (DISPLAY_VER(display) >= 5 || display->platform.g4x) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 367b53a9eae2..62d0785c9edf 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1512,6 +1512,8 @@ struct intel_plane { bool async_flip); void (*enable_flip_done)(struct intel_plane *plane); void (*disable_flip_done)(struct intel_plane *plane); + /* For drm_panic */ + void (*disable_tiling)(struct intel_plane *plane); }; #define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base)
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. Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- drivers/gpu/drm/i915/display/i9xx_plane.c | 23 +++++++++++++++++++ .../drm/i915/display/intel_display_types.h | 2 ++ 2 files changed, 25 insertions(+)