| Message ID | 20240328120638.468738-6-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panic: Add a drm panic handler | expand |
Hi Am 28.03.24 um 13:03 schrieb Jocelyn Falempe: > This was initialy done for imx6, but should work on most drivers > using drm_fb_dma_helper. > > v8: > * Replace get_scanout_buffer() logic with drm_panic_set_buffer() > (Thomas Zimmermann) > > v9: > * go back to get_scanout_buffer() > * move get_scanout_buffer() to plane helper functions > > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > --- > drivers/gpu/drm/drm_fb_dma_helper.c | 47 +++++++++++++++++++++++++++++ > include/drm/drm_fb_dma_helper.h | 4 +++ > 2 files changed, 51 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c > index 3b535ad1b07c..010327069ad4 100644 > --- a/drivers/gpu/drm/drm_fb_dma_helper.c > +++ b/drivers/gpu/drm/drm_fb_dma_helper.c > @@ -15,6 +15,7 @@ > #include <drm/drm_framebuffer.h> > #include <drm/drm_gem_dma_helper.h> > #include <drm/drm_gem_framebuffer_helper.h> > +#include <drm/drm_panic.h> > #include <drm/drm_plane.h> > #include <linux/dma-mapping.h> > #include <linux/module.h> > @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, > } > } > EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); > + > +#if defined(CONFIG_DRM_PANIC) I would not bother with CONFIG_DRM_PANIC for now and make the helper generally available. > +/** > + * @plane: DRM primary plane > + * @drm_scanout_buffer: scanout buffer for the panic handler > + * Returns: 0 or negative error code > + * > + * Generic get_scanout_buffer() implementation, for drivers that uses the > + * drm_fb_dma_helper. > + */ > +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, > + struct drm_scanout_buffer *sb) It's neither really a function for panic handling nor a GEM function. This helper needs to be called drm_fb_dma_get_scanout_buffer() IMHO. > +{ > + struct drm_gem_dma_object *dma_obj; > + struct drm_framebuffer *fb; > + > + fb = plane->state->fb; > + /* Only support linear modifier */ > + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) > + return -ENODEV; > + > + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); > + > + /* Buffer should be accessible from the CPU */ > + if (dma_obj->base.import_attach) > + return -ENODEV; > + > + /* Buffer should be already mapped to CPU */ > + if (!dma_obj->vaddr) > + return -ENODEV; > + > + iosys_map_set_vaddr(&sb->map, dma_obj->vaddr); > + sb->format = fb->format; > + sb->height = fb->height; > + sb->width = fb->width; > + sb->pitch = fb->pitches[0]; > + return 0; > +} > +#else > +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, > + struct drm_scanout_buffer *sb) > +{ > + return -ENODEV; > +} > +#endif > +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); > diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h > index d5e036c57801..61f24c2aba2f 100644 > --- a/include/drm/drm_fb_dma_helper.h > +++ b/include/drm/drm_fb_dma_helper.h > @@ -7,6 +7,7 @@ > struct drm_device; > struct drm_framebuffer; > struct drm_plane_state; > +struct drm_scanout_buffer; > > struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, > unsigned int plane); > @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, > struct drm_plane_state *old_state, > struct drm_plane_state *state); > > +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, > + struct drm_scanout_buffer *sb); > + > #endif >
On 09/04/2024 10:21, Thomas Zimmermann wrote: > Hi > > Am 28.03.24 um 13:03 schrieb Jocelyn Falempe: >> This was initialy done for imx6, but should work on most drivers >> using drm_fb_dma_helper. >> >> v8: >> * Replace get_scanout_buffer() logic with drm_panic_set_buffer() >> (Thomas Zimmermann) >> >> v9: >> * go back to get_scanout_buffer() >> * move get_scanout_buffer() to plane helper functions >> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> --- >> drivers/gpu/drm/drm_fb_dma_helper.c | 47 +++++++++++++++++++++++++++++ >> include/drm/drm_fb_dma_helper.h | 4 +++ >> 2 files changed, 51 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c >> b/drivers/gpu/drm/drm_fb_dma_helper.c >> index 3b535ad1b07c..010327069ad4 100644 >> --- a/drivers/gpu/drm/drm_fb_dma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_dma_helper.c >> @@ -15,6 +15,7 @@ >> #include <drm/drm_framebuffer.h> >> #include <drm/drm_gem_dma_helper.h> >> #include <drm/drm_gem_framebuffer_helper.h> >> +#include <drm/drm_panic.h> >> #include <drm/drm_plane.h> >> #include <linux/dma-mapping.h> >> #include <linux/module.h> >> @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct >> drm_device *drm, >> } >> } >> EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); >> + >> +#if defined(CONFIG_DRM_PANIC) > > I would not bother with CONFIG_DRM_PANIC for now and make the helper > generally available. yes, that's a small function, let's keep it simple. Done in v12 > >> +/** >> + * @plane: DRM primary plane >> + * @drm_scanout_buffer: scanout buffer for the panic handler >> + * Returns: 0 or negative error code >> + * >> + * Generic get_scanout_buffer() implementation, for drivers that uses >> the >> + * drm_fb_dma_helper. >> + */ >> +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, >> + struct drm_scanout_buffer *sb) > > It's neither really a function for panic handling nor a GEM function. > This helper needs to be called drm_fb_dma_get_scanout_buffer() IMHO. > I agree, it matches the other functions prefixes in this file. Done in v12 >> +{ >> + struct drm_gem_dma_object *dma_obj; >> + struct drm_framebuffer *fb; >> + >> + fb = plane->state->fb; >> + /* Only support linear modifier */ >> + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) >> + return -ENODEV; >> + >> + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); >> + >> + /* Buffer should be accessible from the CPU */ >> + if (dma_obj->base.import_attach) >> + return -ENODEV; >> + >> + /* Buffer should be already mapped to CPU */ >> + if (!dma_obj->vaddr) >> + return -ENODEV; >> + >> + iosys_map_set_vaddr(&sb->map, dma_obj->vaddr); >> + sb->format = fb->format; >> + sb->height = fb->height; >> + sb->width = fb->width; >> + sb->pitch = fb->pitches[0]; >> + return 0; >> +} >> +#else >> +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, >> + struct drm_scanout_buffer *sb) >> +{ >> + return -ENODEV; >> +} >> +#endif >> +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); >> diff --git a/include/drm/drm_fb_dma_helper.h >> b/include/drm/drm_fb_dma_helper.h >> index d5e036c57801..61f24c2aba2f 100644 >> --- a/include/drm/drm_fb_dma_helper.h >> +++ b/include/drm/drm_fb_dma_helper.h >> @@ -7,6 +7,7 @@ >> struct drm_device; >> struct drm_framebuffer; >> struct drm_plane_state; >> +struct drm_scanout_buffer; >> struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct >> drm_framebuffer *fb, >> unsigned int plane); >> @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device >> *drm, >> struct drm_plane_state *old_state, >> struct drm_plane_state *state); >> +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, >> + struct drm_scanout_buffer *sb); >> + >> #endif > Thanks for the reviews,
diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c index 3b535ad1b07c..010327069ad4 100644 --- a/drivers/gpu/drm/drm_fb_dma_helper.c +++ b/drivers/gpu/drm/drm_fb_dma_helper.c @@ -15,6 +15,7 @@ #include <drm/drm_framebuffer.h> #include <drm/drm_gem_dma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> +#include <drm/drm_panic.h> #include <drm/drm_plane.h> #include <linux/dma-mapping.h> #include <linux/module.h> @@ -148,3 +149,49 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, } } EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent); + +#if defined(CONFIG_DRM_PANIC) +/** + * @plane: DRM primary plane + * @drm_scanout_buffer: scanout buffer for the panic handler + * Returns: 0 or negative error code + * + * Generic get_scanout_buffer() implementation, for drivers that uses the + * drm_fb_dma_helper. + */ +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + struct drm_gem_dma_object *dma_obj; + struct drm_framebuffer *fb; + + fb = plane->state->fb; + /* Only support linear modifier */ + if (fb->modifier != DRM_FORMAT_MOD_LINEAR) + return -ENODEV; + + dma_obj = drm_fb_dma_get_gem_obj(fb, 0); + + /* Buffer should be accessible from the CPU */ + if (dma_obj->base.import_attach) + return -ENODEV; + + /* Buffer should be already mapped to CPU */ + if (!dma_obj->vaddr) + return -ENODEV; + + iosys_map_set_vaddr(&sb->map, dma_obj->vaddr); + sb->format = fb->format; + sb->height = fb->height; + sb->width = fb->width; + sb->pitch = fb->pitches[0]; + return 0; +} +#else +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb) +{ + return -ENODEV; +} +#endif +EXPORT_SYMBOL(drm_panic_gem_get_scanout_buffer); diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h index d5e036c57801..61f24c2aba2f 100644 --- a/include/drm/drm_fb_dma_helper.h +++ b/include/drm/drm_fb_dma_helper.h @@ -7,6 +7,7 @@ struct drm_device; struct drm_framebuffer; struct drm_plane_state; +struct drm_scanout_buffer; struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb, unsigned int plane); @@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm, struct drm_plane_state *old_state, struct drm_plane_state *state); +int drm_panic_gem_get_scanout_buffer(struct drm_plane *plane, + struct drm_scanout_buffer *sb); + #endif
This was initialy done for imx6, but should work on most drivers using drm_fb_dma_helper. v8: * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) v9: * go back to get_scanout_buffer() * move get_scanout_buffer() to plane helper functions Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- drivers/gpu/drm/drm_fb_dma_helper.c | 47 +++++++++++++++++++++++++++++ include/drm/drm_fb_dma_helper.h | 4 +++ 2 files changed, 51 insertions(+)