From f7aef1c207092770d06d0df21dceafdca2b49c39 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Mon, 30 Apr 2018 15:32:32 +0200 Subject: drm/vc4: Make sure vc4_bo_{inc,dec}_usecnt() calls are balanced Commit b9f19259b84d ("drm/vc4: Add the DRM_IOCTL_VC4_GEM_MADVISE ioctl") introduced a mechanism to mark some BOs as purgeable to allow the driver to drop them under memory pressure. In order to implement this feature we had to add a mechanism to mark BOs as currently used by a piece of hardware which materialized through the ->usecnt counter. Plane code is supposed to increment usecnt when it attaches a BO to a plane and decrement it when it's done with this BO, which was done in the ->prepare_fb() and ->cleanup_fb() hooks. The problem is, async page flip logic does not go through the regular atomic update path, and ->prepare_fb() and ->cleanup_fb() are not called in this case. Fix that by manually calling vc4_bo_{inc,dec}_usecnt() in the async-page-flip path. Note that all this should go away as soon as we get generic async page flip support in the core, in the meantime, this fix should do the trick. Fixes: b9f19259b84d ("drm/vc4: Add the DRM_IOCTL_VC4_GEM_MADVISE ioctl") Reported-by: Peter Robinson Cc: Signed-off-by: Boris Brezillon Signed-off-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20180430133232.32457-1-boris.brezillon@bootlin.com Link: https://patchwork.freedesktop.org/patch/msgid/20180430133232.32457-1-boris.brezillon@bootlin.com --- drivers/gpu/drm/vc4/vc4_crtc.c | 46 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vc4') diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index bf4667481935..c61dff594195 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -760,6 +760,7 @@ static irqreturn_t vc4_crtc_irq_handler(int irq, void *data) struct vc4_async_flip_state { struct drm_crtc *crtc; struct drm_framebuffer *fb; + struct drm_framebuffer *old_fb; struct drm_pending_vblank_event *event; struct vc4_seqno_cb cb; @@ -789,6 +790,23 @@ vc4_async_page_flip_complete(struct vc4_seqno_cb *cb) drm_crtc_vblank_put(crtc); drm_framebuffer_put(flip_state->fb); + + /* Decrement the BO usecnt in order to keep the inc/dec calls balanced + * when the planes are updated through the async update path. + * FIXME: we should move to generic async-page-flip when it's + * available, so that we can get rid of this hand-made cleanup_fb() + * logic. + */ + if (flip_state->old_fb) { + struct drm_gem_cma_object *cma_bo; + struct vc4_bo *bo; + + cma_bo = drm_fb_cma_get_gem_obj(flip_state->old_fb, 0); + bo = to_vc4_bo(&cma_bo->base); + vc4_bo_dec_usecnt(bo); + drm_framebuffer_put(flip_state->old_fb); + } + kfree(flip_state); up(&vc4->async_modeset); @@ -813,9 +831,22 @@ static int vc4_async_page_flip(struct drm_crtc *crtc, struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb, 0); struct vc4_bo *bo = to_vc4_bo(&cma_bo->base); + /* Increment the BO usecnt here, so that we never end up with an + * unbalanced number of vc4_bo_{dec,inc}_usecnt() calls when the + * plane is later updated through the non-async path. + * FIXME: we should move to generic async-page-flip when it's + * available, so that we can get rid of this hand-made prepare_fb() + * logic. + */ + ret = vc4_bo_inc_usecnt(bo); + if (ret) + return ret; + flip_state = kzalloc(sizeof(*flip_state), GFP_KERNEL); - if (!flip_state) + if (!flip_state) { + vc4_bo_dec_usecnt(bo); return -ENOMEM; + } drm_framebuffer_get(fb); flip_state->fb = fb; @@ -826,10 +857,23 @@ static int vc4_async_page_flip(struct drm_crtc *crtc, ret = down_interruptible(&vc4->async_modeset); if (ret) { drm_framebuffer_put(fb); + vc4_bo_dec_usecnt(bo); kfree(flip_state); return ret; } + /* Save the current FB before it's replaced by the new one in + * drm_atomic_set_fb_for_plane(). We'll need the old FB in + * vc4_async_page_flip_complete() to decrement the BO usecnt and keep + * it consistent. + * FIXME: we should move to generic async-page-flip when it's + * available, so that we can get rid of this hand-made cleanup_fb() + * logic. + */ + flip_state->old_fb = plane->state->fb; + if (flip_state->old_fb) + drm_framebuffer_get(flip_state->old_fb); + WARN_ON(drm_crtc_vblank_get(crtc) != 0); /* Immediately update the plane's legacy fb pointer, so that later -- cgit v1.2.3-55-g7522 From 164c2416dd40770aba5814f93da835e8a9f7196d Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 9 Mar 2018 15:32:56 -0800 Subject: drm/vc4: Fix oops dereferencing DPI's connector since panel_bridge. In the cleanup, I didn't notice that we needed to dereference the connector for the bus_format. Fix the regression by looking up the first (and only) connector attached to us, and assume that its bus_format is what we want. Some day it would be good to have that part of display_info attached to the bridge, instead. v2: Fix stray whitespace change Signed-off-by: Eric Anholt Fixes: 7b1298e05310 ("drm/vc4: Switch DPI to using the panel-bridge helper.") Link: https://patchwork.freedesktop.org/patch/msgid/20180309233256.1667-1-eric@anholt.net Reviewed-by: Sean Paul Reviewed-by: Boris Brezillon Signed-off-by: Sean Paul --- drivers/gpu/drm/vc4/vc4_dpi.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/vc4') diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index 72c9dbd81d7f..f185812970da 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -96,7 +96,6 @@ struct vc4_dpi { struct platform_device *pdev; struct drm_encoder *encoder; - struct drm_connector *connector; void __iomem *regs; @@ -164,14 +163,31 @@ static void vc4_dpi_encoder_disable(struct drm_encoder *encoder) static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) { + struct drm_device *dev = encoder->dev; struct drm_display_mode *mode = &encoder->crtc->mode; struct vc4_dpi_encoder *vc4_encoder = to_vc4_dpi_encoder(encoder); struct vc4_dpi *dpi = vc4_encoder->dpi; + struct drm_connector_list_iter conn_iter; + struct drm_connector *connector = NULL, *connector_scan; u32 dpi_c = DPI_ENABLE | DPI_OUTPUT_ENABLE_MODE; int ret; - if (dpi->connector->display_info.num_bus_formats) { - u32 bus_format = dpi->connector->display_info.bus_formats[0]; + /* Look up the connector attached to DPI so we can get the + * bus_format. Ideally the bridge would tell us the + * bus_format we want, but it doesn't yet, so assume that it's + * uniform throughout the bridge chain. + */ + drm_connector_list_iter_begin(dev, &conn_iter); + drm_for_each_connector_iter(connector_scan, &conn_iter) { + if (connector_scan->encoder == encoder) { + connector = connector_scan; + break; + } + } + drm_connector_list_iter_end(&conn_iter); + + if (connector && connector->display_info.num_bus_formats) { + u32 bus_format = connector->display_info.bus_formats[0]; switch (bus_format) { case MEDIA_BUS_FMT_RGB888_1X24: @@ -199,6 +215,9 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) DRM_ERROR("Unknown media bus format %d\n", bus_format); break; } + } else { + /* Default to 24bit if no connector found. */ + dpi_c |= VC4_SET_FIELD(DPI_FORMAT_24BIT_888_RGB, DPI_FORMAT); } if (mode->flags & DRM_MODE_FLAG_NHSYNC) -- cgit v1.2.3-55-g7522 From 9a0e9802217291e54c4dd1fc5462f189a4be14ec Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Mon, 7 May 2018 14:13:03 +0200 Subject: drm/vc4: Fix scaling of uni-planar formats When using uni-planar formats (like RGB), the scaling parameters are stored in plane 0, not plane 1. Fixes: fc04023fafec ("drm/vc4: Add support for YUV planes.") Cc: stable@vger.kernel.org Signed-off-by: Boris Brezillon Reviewed-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20180507121303.5610-1-boris.brezillon@bootlin.com --- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vc4') diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index ce39390be389..13dcaad06798 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -503,7 +503,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane, * the scl fields here. */ if (num_planes == 1) { - scl0 = vc4_get_scl_field(state, 1); + scl0 = vc4_get_scl_field(state, 0); scl1 = scl0; } else { scl0 = vc4_get_scl_field(state, 1); -- cgit v1.2.3-55-g7522 From f619794d716c0615241b73a0a5e78dbc8442de7b Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Mon, 9 Apr 2018 13:58:13 -0700 Subject: drm/vc4: Fix leak of the file_priv that stored the perfmon. Signed-off-by: Eric Anholt Fixes: 65101d8c9108 ("drm/vc4: Expose performance counters to userspace") Link: https://patchwork.freedesktop.org/patch/msgid/20180409205813.7077-1-eric@anholt.net Reviewed-by: Boris Brezillon --- drivers/gpu/drm/vc4/vc4_drv.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/vc4') diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index d9b8b701d2ce..466d0a27b415 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -130,6 +130,7 @@ static void vc4_close(struct drm_device *dev, struct drm_file *file) struct vc4_file *vc4file = file->driver_priv; vc4_perfmon_close_file(vc4file); + kfree(vc4file); } static const struct vm_operations_struct vc4_vm_ops = { -- cgit v1.2.3-55-g7522