[PATCH 1/5] drm/imx: don't call disable_plane in plane destroy path

[PATCH 1/5] drm/imx: don't call disable_plane in plane destroy path

[Lucas Stach]

When the destroy path is called the plane should already be
disabled. If not, this is a core bug and should not be worked
around in the driver.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 4ad67d015ec7..6cc809e1ca55 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -242,7 +242,6 @@ static void ipu_plane_destroy(struct drm_plane *plane)
 
 	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
 
-	ipu_disable_plane(plane);
 	drm_plane_cleanup(plane);
 	kfree(ipu_plane);
 }
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] drm/imx: drop deprecated load/unload drm_driver ops

[Lucas Stach]

Drop the load/unload driver ops, as they are deprecated because of their
inherent races, with devices being visible to userspace before they are
fully initialized.

Move this code into the driver bind/unbind routines bracketed by the
proper drm_dev_alloc/register and drm_dev_unregister/unref calls.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/imx/imx-drm-core.c | 240 +++++++++++++++++--------------------
 1 file changed, 112 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 9f7dafce3a4c..c0d06a0f6825 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -71,25 +71,6 @@ static void imx_drm_driver_lastclose(struct drm_device *drm)
 	drm_fbdev_cma_restore_mode(imxdrm->fbhelper);
 }
 
-static int imx_drm_driver_unload(struct drm_device *drm)
-{
-	struct imx_drm_device *imxdrm = drm->dev_private;
-
-	drm_kms_helper_poll_fini(drm);
-
-	if (imxdrm->fbhelper)
-		drm_fbdev_cma_fini(imxdrm->fbhelper);
-
-	component_unbind_all(drm->dev, drm);
-
-	drm_vblank_cleanup(drm);
-	drm_mode_config_cleanup(drm);
-
-	platform_set_drvdata(drm->platformdev, NULL);
-
-	return 0;
-}
-
 int imx_drm_crtc_vblank_get(struct imx_drm_crtc *imx_drm_crtc)
 {
 	return drm_crtc_vblank_get(imx_drm_crtc->crtc);
@@ -234,111 +215,6 @@ static struct drm_mode_config_helper_funcs imx_drm_mode_config_helpers = {
 };
 
 /*
- * Main DRM initialisation. This binds, initialises and registers
- * with DRM the subcomponents of the driver.
- */
-static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags)
-{
-	struct imx_drm_device *imxdrm;
-	struct drm_connector *connector;
-	int ret;
-
-	imxdrm = devm_kzalloc(drm->dev, sizeof(*imxdrm), GFP_KERNEL);
-	if (!imxdrm)
-		return -ENOMEM;
-
-	imxdrm->drm = drm;
-
-	drm->dev_private = imxdrm;
-
-	/*
-	 * enable drm irq mode.
-	 * - with irq_enabled = true, we can use the vblank feature.
-	 *
-	 * P.S. note that we wouldn't use drm irq handler but
-	 *      just specific driver own one instead because
-	 *      drm framework supports only one irq handler and
-	 *      drivers can well take care of their interrupts
-	 */
-	drm->irq_enabled = true;
-
-	/*
-	 * set max width and height as default value(4096x4096).
-	 * this value would be used to check framebuffer size limitation
-	 * at drm_mode_addfb().
-	 */
-	drm->mode_config.min_width = 64;
-	drm->mode_config.min_height = 64;
-	drm->mode_config.max_width = 4096;
-	drm->mode_config.max_height = 4096;
-	drm->mode_config.funcs = &imx_drm_mode_config_funcs;
-	drm->mode_config.helper_private = &imx_drm_mode_config_helpers;
-
-	drm_mode_config_init(drm);
-
-	ret = drm_vblank_init(drm, MAX_CRTC);
-	if (ret)
-		goto err_kms;
-
-	platform_set_drvdata(drm->platformdev, drm);
-
-	/* Now try and bind all our sub-components */
-	ret = component_bind_all(drm->dev, drm);
-	if (ret)
-		goto err_vblank;
-
-	/*
-	 * All components are now added, we can publish the connector sysfs
-	 * entries to userspace.  This will generate hotplug events and so
-	 * userspace will expect to be able to access DRM at this point.
-	 */
-	list_for_each_entry(connector, &drm->mode_config.connector_list, head) {
-		ret = drm_connector_register(connector);
-		if (ret) {
-			dev_err(drm->dev,
-				"[CONNECTOR:%d:%s] drm_connector_register failed: %d\n",
-				connector->base.id,
-				connector->name, ret);
-			goto err_unbind;
-		}
-	}
-
-	drm_mode_config_reset(drm);
-
-	/*
-	 * All components are now initialised, so setup the fb helper.
-	 * The fb helper takes copies of key hardware information, so the
-	 * crtcs/connectors/encoders must not change after this point.
-	 */
-#if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
-	if (legacyfb_depth != 16 && legacyfb_depth != 32) {
-		dev_warn(drm->dev, "Invalid legacyfb_depth.  Defaulting to 16bpp\n");
-		legacyfb_depth = 16;
-	}
-	imxdrm->fbhelper = drm_fbdev_cma_init(drm, legacyfb_depth,
-				drm->mode_config.num_crtc, MAX_CRTC);
-	if (IS_ERR(imxdrm->fbhelper)) {
-		ret = PTR_ERR(imxdrm->fbhelper);
-		imxdrm->fbhelper = NULL;
-		goto err_unbind;
-	}
-#endif
-
-	drm_kms_helper_poll_init(drm);
-
-	return 0;
-
-err_unbind:
-	component_unbind_all(drm->dev, drm);
-err_vblank:
-	drm_vblank_cleanup(drm);
-err_kms:
-	drm_mode_config_cleanup(drm);
-
-	return ret;
-}
-
-/*
  * imx_drm_add_crtc - add a new crtc
  */
 int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
@@ -430,8 +306,6 @@ static const struct drm_ioctl_desc imx_drm_ioctls[] = {
 static struct drm_driver imx_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
-	.load			= imx_drm_driver_load,
-	.unload			= imx_drm_driver_unload,
 	.lastclose		= imx_drm_driver_lastclose,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops		= &drm_gem_cma_vm_ops,
@@ -484,12 +358,122 @@ static int compare_of(struct device *dev, void *data)
 
 static int imx_drm_bind(struct device *dev)
 {
-	return drm_platform_init(&imx_drm_driver, to_platform_device(dev));
+	struct drm_device *drm;
+	struct imx_drm_device *imxdrm;
+	int ret;
+
+	drm = drm_dev_alloc(&imx_drm_driver, dev);
+	if (!drm)
+		return -ENOMEM;
+
+	imxdrm = devm_kzalloc(dev, sizeof(*imxdrm), GFP_KERNEL);
+	if (!imxdrm) {
+		ret = -ENOMEM;
+		goto err_unref;
+	}
+
+	imxdrm->drm = drm;
+	drm->dev_private = imxdrm;
+
+	/*
+	 * enable drm irq mode.
+	 * - with irq_enabled = true, we can use the vblank feature.
+	 *
+	 * P.S. note that we wouldn't use drm irq handler but
+	 *      just specific driver own one instead because
+	 *      drm framework supports only one irq handler and
+	 *      drivers can well take care of their interrupts
+	 */
+	drm->irq_enabled = true;
+
+	/*
+	 * set max width and height as default value(4096x4096).
+	 * this value would be used to check framebuffer size limitation
+	 * at drm_mode_addfb().
+	 */
+	drm->mode_config.min_width = 64;
+	drm->mode_config.min_height = 64;
+	drm->mode_config.max_width = 4096;
+	drm->mode_config.max_height = 4096;
+	drm->mode_config.funcs = &imx_drm_mode_config_funcs;
+	drm->mode_config.helper_private = &imx_drm_mode_config_helpers;
+
+	drm_mode_config_init(drm);
+
+	ret = drm_vblank_init(drm, MAX_CRTC);
+	if (ret)
+		goto err_kms;
+
+	dev_set_drvdata(dev, drm);
+
+	/* Now try and bind all our sub-components */
+	ret = component_bind_all(dev, drm);
+	if (ret)
+		goto err_vblank;
+
+	drm_mode_config_reset(drm);
+
+	/*
+	 * All components are now initialised, so setup the fb helper.
+	 * The fb helper takes copies of key hardware information, so the
+	 * crtcs/connectors/encoders must not change after this point.
+	 */
+#if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
+	if (legacyfb_depth != 16 && legacyfb_depth != 32) {
+		dev_warn(dev, "Invalid legacyfb_depth.  Defaulting to 16bpp\n");
+		legacyfb_depth = 16;
+	}
+	imxdrm->fbhelper = drm_fbdev_cma_init(drm, legacyfb_depth,
+				drm->mode_config.num_crtc, MAX_CRTC);
+	if (IS_ERR(imxdrm->fbhelper)) {
+		ret = PTR_ERR(imxdrm->fbhelper);
+		imxdrm->fbhelper = NULL;
+		goto err_unbind;
+	}
+#endif
+
+	drm_kms_helper_poll_init(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		goto err_fbhelper;
+
+	return 0;
+
+err_fbhelper:
+	drm_kms_helper_poll_fini(drm);
+	if (imxdrm->fbhelper)
+		drm_fbdev_cma_fini(imxdrm->fbhelper);
+err_unbind:
+	component_unbind_all(drm->dev, drm);
+err_vblank:
+	drm_vblank_cleanup(drm);
+err_kms:
+	drm_mode_config_cleanup(drm);
+err_unref:
+	drm_dev_unref(drm);
+
+	return ret;
 }
 
 static void imx_drm_unbind(struct device *dev)
 {
-	drm_put_dev(dev_get_drvdata(dev));
+	struct drm_device *drm = dev_get_drvdata(dev);
+	struct imx_drm_device *imxdrm = drm->dev_private;
+
+	drm_dev_unregister(drm);
+
+	drm_kms_helper_poll_fini(drm);
+
+	if (imxdrm->fbhelper)
+		drm_fbdev_cma_fini(imxdrm->fbhelper);
+
+	component_unbind_all(drm->dev, drm);
+	dev_set_drvdata(dev, NULL);
+
+	drm_mode_config_cleanup(drm);
+
+	drm_dev_unref(drm);
 }
 
 static const struct component_master_ops imx_drm_ops = {
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm/imx: don't destroy mode objects manually on driver unbind

[Lucas Stach]

Instead let drm_mode_config_cleanup() do the work when taking down
the master device. This requires all cleanup functions to be
properly hooked up to the mode object .destroy callback.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/bridge/dw-hdmi.c       | 3 ---
 drivers/gpu/drm/imx/imx-drm-core.c     | 4 ++--
 drivers/gpu/drm/imx/imx-ldb.c          | 6 ------
 drivers/gpu/drm/imx/imx-tve.c          | 3 ---
 drivers/gpu/drm/imx/ipuv3-crtc.c       | 9 ++++++---
 drivers/gpu/drm/imx/parallel-display.c | 3 ---
 6 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 77ab47341658..56f3d86a4be6 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1812,9 +1812,6 @@ void dw_hdmi_unbind(struct device *dev, struct device *master, void *data)
 	/* Disable all interrupts */
 	hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
 
-	hdmi->connector.funcs->destroy(&hdmi->connector);
-	hdmi->encoder->funcs->destroy(hdmi->encoder);
-
 	clk_disable_unprepare(hdmi->iahb_clk);
 	clk_disable_unprepare(hdmi->isfr_clk);
 	i2c_put_adapter(hdmi->ddc);
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index c0d06a0f6825..66b3556f7b79 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -468,11 +468,11 @@ static void imx_drm_unbind(struct device *dev)
 	if (imxdrm->fbhelper)
 		drm_fbdev_cma_fini(imxdrm->fbhelper);
 
+	drm_mode_config_cleanup(drm);
+
 	component_unbind_all(drm->dev, drm);
 	dev_set_drvdata(dev, NULL);
 
-	drm_mode_config_cleanup(drm);
-
 	drm_dev_unref(drm);
 }
 
diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index b03919ed60ba..1c091dd97688 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -716,12 +716,6 @@ static void imx_ldb_unbind(struct device *dev, struct device *master,
 	for (i = 0; i < 2; i++) {
 		struct imx_ldb_channel *channel = &imx_ldb->channel[i];
 
-		if (!channel->connector.funcs)
-			continue;
-
-		channel->connector.funcs->destroy(&channel->connector);
-		channel->encoder.funcs->destroy(&channel->encoder);
-
 		kfree(channel->edid);
 		i2c_put_adapter(channel->ddc);
 	}
diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 5e875944ffa2..8fc088843e55 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -685,9 +685,6 @@ static void imx_tve_unbind(struct device *dev, struct device *master,
 {
 	struct imx_tve *tve = dev_get_drvdata(dev);
 
-	tve->connector.funcs->destroy(&tve->connector);
-	tve->encoder.funcs->destroy(&tve->encoder);
-
 	if (!IS_ERR(tve->dac_reg))
 		regulator_disable(tve->dac_reg);
 }
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 08e188bc10fc..a2a239fe0dba 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -121,9 +121,14 @@ static void imx_drm_crtc_destroy_state(struct drm_crtc *crtc,
 	kfree(to_imx_crtc_state(state));
 }
 
+static void imx_drm_crtc_destroy(struct drm_crtc *crtc)
+{
+	imx_drm_remove_crtc(to_ipu_crtc(crtc)->imx_crtc);
+}
+
 static const struct drm_crtc_funcs ipu_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
-	.destroy = drm_crtc_cleanup,
+	.destroy = imx_drm_crtc_destroy,
 	.page_flip = drm_atomic_helper_page_flip,
 	.reset = imx_drm_crtc_reset,
 	.atomic_duplicate_state = imx_drm_crtc_duplicate_state,
@@ -410,8 +415,6 @@ static void ipu_drm_unbind(struct device *dev, struct device *master,
 {
 	struct ipu_crtc *ipu_crtc = dev_get_drvdata(dev);
 
-	imx_drm_remove_crtc(ipu_crtc->imx_crtc);
-
 	ipu_put_resources(ipu_crtc);
 	if (ipu_crtc->plane[1])
 		ipu_plane_put_resources(ipu_crtc->plane[1]);
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index 1dad297b01fd..d6010d09ade0 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -289,9 +289,6 @@ static void imx_pd_unbind(struct device *dev, struct device *master,
 {
 	struct imx_parallel_display *imxpd = dev_get_drvdata(dev);
 
-	imxpd->encoder.funcs->destroy(&imxpd->encoder);
-	imxpd->connector.funcs->destroy(&imxpd->connector);
-
 	kfree(imxpd->edid);
 }
 
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/imx: fold ipu_plane_disable into ipu_disable_plane

[Lucas Stach]

ipu_disable_plane is the only left caller of ipu_plane_disable.
Having those 2 similar named functions is confusing and superfluous,
so fold them into 1.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 6cc809e1ca55..a6247f57541d 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -213,8 +213,12 @@ static void ipu_plane_enable(struct ipu_plane *ipu_plane)
 		ipu_dp_enable_channel(ipu_plane->dp);
 }
 
-static void ipu_plane_disable(struct ipu_plane *ipu_plane)
+static int ipu_disable_plane(struct drm_plane *plane)
 {
+	struct ipu_plane *ipu_plane = to_ipu_plane(plane);
+
+	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
+
 	ipu_idmac_wait_busy(ipu_plane->ipu_ch, 50);
 
 	if (ipu_plane->dp)
@@ -223,15 +227,6 @@ static void ipu_plane_disable(struct ipu_plane *ipu_plane)
 	ipu_dmfc_disable_channel(ipu_plane->dmfc);
 	if (ipu_plane->dp)
 		ipu_dp_disable(ipu_plane->ipu);
-}
-
-static int ipu_disable_plane(struct drm_plane *plane)
-{
-	struct ipu_plane *ipu_plane = to_ipu_plane(plane);
-
-	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
-
-	ipu_plane_disable(ipu_plane);
 
 	return 0;
 }
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] drm/imx: add exclusive fence to plane state

[Lucas Stach]

This allows the atomic helper to wait on them, instead of open-coding
the same in the imx-drm driver.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/imx/imx-drm-core.c | 63 +++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 66b3556f7b79..2d3f32f1b13b 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -152,50 +152,43 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
 	drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
 }
 
+static int imx_drm_atomic_commit(struct drm_device *dev,
+				 struct drm_atomic_state *state,
+				 bool nonblock)
+{
+	struct drm_plane_state *plane_state;
+	struct drm_plane *plane;
+	struct dma_buf *dma_buf;
+	int i;
+
+	/*
+	 * If the plane fb has an dma-buf attached, fish out the exclusive
+	 * fence for the atomic helper to wait on.
+	 */
+	for_each_plane_in_state(state, plane, plane_state, i) {
+		if ((plane->state->fb != plane_state->fb) && plane_state->fb) {
+			dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb,
+							 0)->base.dma_buf;
+			if (!dma_buf)
+				continue;
+			plane_state->fence =
+				reservation_object_get_excl_rcu(dma_buf->resv);
+		}
+	}
+
+	return drm_atomic_helper_commit(dev, state, nonblock);
+}
+
 static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
 	.fb_create = drm_fb_cma_create,
 	.output_poll_changed = imx_drm_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
-	.atomic_commit = drm_atomic_helper_commit,
+	.atomic_commit = imx_drm_atomic_commit,
 };
 
 static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
-	struct drm_plane_state *plane_state;
-	struct drm_gem_cma_object *cma_obj;
-	struct fence *excl;
-	unsigned shared_count;
-	struct fence **shared;
-	unsigned int i, j;
-	int ret;
-
-	/* Wait for fences. */
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		plane_state = crtc->primary->state;
-		if (plane_state->fb) {
-			cma_obj = drm_fb_cma_get_gem_obj(plane_state->fb, 0);
-			if (cma_obj->base.dma_buf) {
-				ret = reservation_object_get_fences_rcu(
-					cma_obj->base.dma_buf->resv, &excl,
-					&shared_count, &shared);
-				if (unlikely(ret))
-					DRM_ERROR("failed to get fences "
-						  "for buffer\n");
-
-				if (excl) {
-					fence_wait(excl, false);
-					fence_put(excl);
-				}
-				for (j = 0; j < shared_count; i++) {
-					fence_wait(shared[j], false);
-					fence_put(shared[j]);
-				}
-			}
-		}
-	}
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/imx: add exclusive fence to plane state

[Daniel Vetter]

On Thu, Aug 11, 2016 at 11:18 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> This allows the atomic helper to wait on them, instead of open-coding
> the same in the imx-drm driver.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/gpu/drm/imx/imx-drm-core.c | 63 +++++++++++++++++---------------------
>  1 file changed, 28 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> index 66b3556f7b79..2d3f32f1b13b 100644
> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> @@ -152,50 +152,43 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
>         drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
>  }
>
> +static int imx_drm_atomic_commit(struct drm_device *dev,
> +                                struct drm_atomic_state *state,
> +                                bool nonblock)
> +{
> +       struct drm_plane_state *plane_state;
> +       struct drm_plane *plane;
> +       struct dma_buf *dma_buf;
> +       int i;
> +
> +       /*
> +        * If the plane fb has an dma-buf attached, fish out the exclusive
> +        * fence for the atomic helper to wait on.
> +        */
> +       for_each_plane_in_state(state, plane, plane_state, i) {
> +               if ((plane->state->fb != plane_state->fb) && plane_state->fb) {
> +                       dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb,
> +                                                        0)->base.dma_buf;
> +                       if (!dma_buf)
> +                               continue;
> +                       plane_state->fence =
> +                               reservation_object_get_excl_rcu(dma_buf->resv);
> +               }
> +       }

Just an fyi, but the idea was that you could do this in youre
prepare_plane hook, assuming that only when userspace flips is it
interested in syncing (for backwards compat reasons). But this gets
the job done too.

The upshot of doing this consistently in a prepare_plane hook is that
it would then allow us to have a shared implementation for all cma
based kms drivers. If you feel bored, I'd be happy to take a look at a
refactor patch which would extract that helper (and wire it up for
imx).

Just a drive-by comment since I stumbled over this as a conflict
between -next and -fixes when rebuilding intel trees.
-Daniel

> +
> +       return drm_atomic_helper_commit(dev, state, nonblock);
> +}
> +
>  static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
>         .fb_create = drm_fb_cma_create,
>         .output_poll_changed = imx_drm_output_poll_changed,
>         .atomic_check = drm_atomic_helper_check,
> -       .atomic_commit = drm_atomic_helper_commit,
> +       .atomic_commit = imx_drm_atomic_commit,
>  };
>
>  static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
>  {
>         struct drm_device *dev = state->dev;
> -       struct drm_crtc *crtc;
> -       struct drm_crtc_state *crtc_state;
> -       struct drm_plane_state *plane_state;
> -       struct drm_gem_cma_object *cma_obj;
> -       struct fence *excl;
> -       unsigned shared_count;
> -       struct fence **shared;
> -       unsigned int i, j;
> -       int ret;
> -
> -       /* Wait for fences. */
> -       for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -               plane_state = crtc->primary->state;
> -               if (plane_state->fb) {
> -                       cma_obj = drm_fb_cma_get_gem_obj(plane_state->fb, 0);
> -                       if (cma_obj->base.dma_buf) {
> -                               ret = reservation_object_get_fences_rcu(
> -                                       cma_obj->base.dma_buf->resv, &excl,
> -                                       &shared_count, &shared);
> -                               if (unlikely(ret))
> -                                       DRM_ERROR("failed to get fences "
> -                                                 "for buffer\n");
> -
> -                               if (excl) {
> -                                       fence_wait(excl, false);
> -                                       fence_put(excl);
> -                               }
> -                               for (j = 0; j < shared_count; i++) {
> -                                       fence_wait(shared[j], false);
> -                                       fence_put(shared[j]);
> -                               }
> -                       }
> -               }
> -       }
>
>         drm_atomic_helper_commit_modeset_disables(dev, state);
>
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/imx: add exclusive fence to plane state

[Lucas Stach]

Am Montag, den 19.09.2016, 22:48 +0200 schrieb Daniel Vetter:
> On Thu, Aug 11, 2016 at 11:18 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > This allows the atomic helper to wait on them, instead of open-coding
> > the same in the imx-drm driver.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/gpu/drm/imx/imx-drm-core.c | 63 +++++++++++++++++---------------------
> >  1 file changed, 28 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> > index 66b3556f7b79..2d3f32f1b13b 100644
> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> > @@ -152,50 +152,43 @@ static void imx_drm_output_poll_changed(struct drm_device *drm)
> >         drm_fbdev_cma_hotplug_event(imxdrm->fbhelper);
> >  }
> >
> > +static int imx_drm_atomic_commit(struct drm_device *dev,
> > +                                struct drm_atomic_state *state,
> > +                                bool nonblock)
> > +{
> > +       struct drm_plane_state *plane_state;
> > +       struct drm_plane *plane;
> > +       struct dma_buf *dma_buf;
> > +       int i;
> > +
> > +       /*
> > +        * If the plane fb has an dma-buf attached, fish out the exclusive
> > +        * fence for the atomic helper to wait on.
> > +        */
> > +       for_each_plane_in_state(state, plane, plane_state, i) {
> > +               if ((plane->state->fb != plane_state->fb) && plane_state->fb) {
> > +                       dma_buf = drm_fb_cma_get_gem_obj(plane_state->fb,
> > +                                                        0)->base.dma_buf;
> > +                       if (!dma_buf)
> > +                               continue;
> > +                       plane_state->fence =
> > +                               reservation_object_get_excl_rcu(dma_buf->resv);
> > +               }
> > +       }
> 
> Just an fyi, but the idea was that you could do this in youre
> prepare_plane hook, assuming that only when userspace flips is it
> interested in syncing (for backwards compat reasons). But this gets
> the job done too.
> 
> The upshot of doing this consistently in a prepare_plane hook is that
> it would then allow us to have a shared implementation for all cma
> based kms drivers. If you feel bored, I'd be happy to take a look at a
> refactor patch which would extract that helper (and wire it up for
> imx).
> 
Thanks for letting me know.

As our original flight to XDC has been canceled and we are re-routed to
the evening flight the boredom might actually kick in. :)

Regards,
Lucas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/imx: don't call disable_plane in plane destroy path

[Philipp Zabel]

Am Donnerstag, den 11.08.2016, 11:18 +0200 schrieb Lucas Stach:
> When the destroy path is called the plane should already be
> disabled. If not, this is a core bug and should not be worked
> around in the driver.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/gpu/drm/imx/ipuv3-plane.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index 4ad67d015ec7..6cc809e1ca55 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -242,7 +242,6 @@ static void ipu_plane_destroy(struct drm_plane *plane)
>  
>  	DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>  
> -	ipu_disable_plane(plane);
>  	drm_plane_cleanup(plane);
>  	kfree(ipu_plane);
>  }

Applied all 5.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel