Den 05.10.2016 15:22, skrev Laurent Pinchart:
Hi Noralf,
Thank you for the patch.
On Sunday 11 Sep 2016 20:47:41 Noralf Trønnes wrote:
This enables panic message output for fb cma helper framebuffers.
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
---
drivers/gpu/drm/drm_fb_cma_helper.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
b/drivers/gpu/drm/drm_fb_cma_helper.c index 1fd6eac..2f1b012 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -126,9 +126,19 @@ int drm_fb_cma_create_handle(struct drm_framebuffer
*fb, }
EXPORT_SYMBOL(drm_fb_cma_create_handle);
+static void *drm_fb_cma_panic_vmap(struct drm_framebuffer *fb)
+{
+ struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+ struct drm_gem_cma_object *cma_obj = fb_cma->obj[0];
+
+ /* A PRIME imported buffer will not have it's vaddr set. */
Does this mean that, if the framebuffer that happens to be displayed when a
panic occurs is imported, no message will be printed ? I understand how
supporting such cases is difficult, but I'm wondering how we could proceed to
ensure that a panic can be displayed in most (if not all) cases.
If we can vmap all cma buffers, then it's simple:
- Add dma_buf_vmap() call to drm_gem_cma_prime_import_sg_table()
- Add dma_buf_vunmap() call to drm_gem_cma_free_object()
If not then it looks more complicated, since this is atomic context.
There is dma_buf_kmap_atomic(), but there are no users.
And drm_gem_prime_dmabuf_ops doesn't support .kmap_atomic either
(returns NULL).
omapdrm is the only dma_buf provider I can find that actually uses
kmap_atomic() instead of just returning NULL or relying on an existing
virtual address. It has it's own .gem_prime_import/export functions to
accomplish this.
Similarly, it looks like your code only handles single-planar formats, but
there's no explicit check for that in patch 1/3. The easiest fix is to reject
multi-planar framebuffers, but that would again result in silent panics in
some cases.
That's correct if we talk about the default panic_draw_xy() function:
drm_framebuffer_panic_draw_xy(). Most likely this function can be extended
to support multi-planar formats, but Daniel said we could wait with that.
And the driver can also implement it's own .panic_draw_xy() function.
+ return cma_obj ? cma_obj->vaddr : NULL;
Can cma_obj be NULL here ? I thought that framebuffer objects were always
created with at least one GEM object.
I was trying to be very careful since a panic is about something gone
very wrong. But in that case I should have checked that fb_cma isn't NULL
also before dereferencing it.
Maybe I'm over the top paranoid :-)
Noralf.
+}
+
static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
.destroy = drm_fb_cma_destroy,
.create_handle = drm_fb_cma_create_handle,
+ .panic_vmap = drm_fb_cma_panic_vmap,
};
static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
