All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] drm/fsl-dcu: initialization fixes
@ 2016-10-17 21:33 ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel
  Cc: alison.wang, jianwei.wang.chn, linux-kernel, Stefan Agner

Hi All,

This is an assortment of fixes which solve issues seen using an
external RGB converter and makes sure that pixel clock is only
on when really required.

Meng, could you test that on a LS1021a?

--
Stefan

Changes since v2:
- Don't enable TCON bypass in case TCON is not available

Changes since v1:
- add patch to no not transfer registers in mode_set_nofb
- add patch which only init fbdev if required
- remove disable unprepare pixel clock on module remove (already disabled
  in CRTC disable callback).
- remove unused label

Stefan Agner (5):
  drm/fsl-dcu: enable TCON bypass mode by default
  drm/fsl-dcu: do not transfer registers on plane init
  drm/fsl-dcu: do not transfer registers in mode_set_nofb
  drm/fsl-dcu: enable pixel clock when enabling CRTC
  drm/fsl-dcu: only init fbdev if required

 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  |  4 +--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 26 ++++---------------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c |  5 ----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c   | 39 ++++-------------------------
 4 files changed, 12 insertions(+), 62 deletions(-)

-- 
2.10.0

^ permalink raw reply	[flat|nested] 21+ messages in thread
* [PATCH v3 0/5] drm/fsl-dcu: initialization fixes
@ 2016-10-17 21:33 ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel; +Cc: jianwei.wang.chn, linux-kernel, alison.wang

Hi All,

This is an assortment of fixes which solve issues seen using an
external RGB converter and makes sure that pixel clock is only
on when really required.

Meng, could you test that on a LS1021a?

--
Stefan

Changes since v2:
- Don't enable TCON bypass in case TCON is not available

Changes since v1:
- add patch to no not transfer registers in mode_set_nofb
- add patch which only init fbdev if required
- remove disable unprepare pixel clock on module remove (already disabled
  in CRTC disable callback).
- remove unused label

Stefan Agner (5):
  drm/fsl-dcu: enable TCON bypass mode by default
  drm/fsl-dcu: do not transfer registers on plane init
  drm/fsl-dcu: do not transfer registers in mode_set_nofb
  drm/fsl-dcu: enable pixel clock when enabling CRTC
  drm/fsl-dcu: only init fbdev if required

 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  |  4 +--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 26 ++++---------------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c |  5 ----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c   | 39 ++++-------------------------
 4 files changed, 12 insertions(+), 62 deletions(-)

-- 
2.10.0

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

^ permalink raw reply	[flat|nested] 21+ messages in thread
* [PATCH v3 1/5] drm/fsl-dcu: enable TCON bypass mode by default
  2016-10-17 21:33 ` Stefan Agner
@ 2016-10-17 21:33   ` Stefan Agner
  -1 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel
  Cc: alison.wang, jianwei.wang.chn, linux-kernel, Stefan Agner

Do not use encoder disable/enable callbacks to control bypass
mode as this seems to mess with the signals not liked by
displays. This also makes more sense since the encoder is
already defined to be parallel RGB/LVDS at creation time.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c |  2 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 39 ++++---------------------------
 2 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 0884c45..3897f56 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -273,6 +273,8 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 		goto disable_dcu_clk;
 	}
 
+	if (fsl_dev->tcon)
+		fsl_tcon_bypass_enable(fsl_dev->tcon);
 	fsl_dcu_drm_init_planes(fsl_dev->drm);
 	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
index 26edcc8..e1dd75b 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
@@ -20,38 +20,6 @@
 #include "fsl_dcu_drm_drv.h"
 #include "fsl_tcon.h"
 
-static int
-fsl_dcu_drm_encoder_atomic_check(struct drm_encoder *encoder,
-				 struct drm_crtc_state *crtc_state,
-				 struct drm_connector_state *conn_state)
-{
-	return 0;
-}
-
-static void fsl_dcu_drm_encoder_disable(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
-
-	if (fsl_dev->tcon)
-		fsl_tcon_bypass_disable(fsl_dev->tcon);
-}
-
-static void fsl_dcu_drm_encoder_enable(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
-
-	if (fsl_dev->tcon)
-		fsl_tcon_bypass_enable(fsl_dev->tcon);
-}
-
-static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
-	.atomic_check = fsl_dcu_drm_encoder_atomic_check,
-	.disable = fsl_dcu_drm_encoder_disable,
-	.enable = fsl_dcu_drm_encoder_enable,
-};
-
 static void fsl_dcu_drm_encoder_destroy(struct drm_encoder *encoder)
 {
 	drm_encoder_cleanup(encoder);
@@ -68,13 +36,16 @@ int fsl_dcu_drm_encoder_create(struct fsl_dcu_drm_device *fsl_dev,
 	int ret;
 
 	encoder->possible_crtcs = 1;
+
+	/* Use bypass mode for parallel RGB/LVDS encoder */
+	if (fsl_dev->tcon)
+		fsl_tcon_bypass_enable(fsl_dev->tcon);
+
 	ret = drm_encoder_init(fsl_dev->drm, encoder, &encoder_funcs,
 			       DRM_MODE_ENCODER_LVDS, NULL);
 	if (ret < 0)
 		return ret;
 
-	drm_encoder_helper_add(encoder, &encoder_helper_funcs);
-
 	return 0;
 }
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 1/5] drm/fsl-dcu: enable TCON bypass mode by default
@ 2016-10-17 21:33   ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel; +Cc: jianwei.wang.chn, linux-kernel, alison.wang

Do not use encoder disable/enable callbacks to control bypass
mode as this seems to mess with the signals not liked by
displays. This also makes more sense since the encoder is
already defined to be parallel RGB/LVDS at creation time.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c |  2 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 39 ++++---------------------------
 2 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 0884c45..3897f56 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -273,6 +273,8 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 		goto disable_dcu_clk;
 	}
 
+	if (fsl_dev->tcon)
+		fsl_tcon_bypass_enable(fsl_dev->tcon);
 	fsl_dcu_drm_init_planes(fsl_dev->drm);
 	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
index 26edcc8..e1dd75b 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
@@ -20,38 +20,6 @@
 #include "fsl_dcu_drm_drv.h"
 #include "fsl_tcon.h"
 
-static int
-fsl_dcu_drm_encoder_atomic_check(struct drm_encoder *encoder,
-				 struct drm_crtc_state *crtc_state,
-				 struct drm_connector_state *conn_state)
-{
-	return 0;
-}
-
-static void fsl_dcu_drm_encoder_disable(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
-
-	if (fsl_dev->tcon)
-		fsl_tcon_bypass_disable(fsl_dev->tcon);
-}
-
-static void fsl_dcu_drm_encoder_enable(struct drm_encoder *encoder)
-{
-	struct drm_device *dev = encoder->dev;
-	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
-
-	if (fsl_dev->tcon)
-		fsl_tcon_bypass_enable(fsl_dev->tcon);
-}
-
-static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
-	.atomic_check = fsl_dcu_drm_encoder_atomic_check,
-	.disable = fsl_dcu_drm_encoder_disable,
-	.enable = fsl_dcu_drm_encoder_enable,
-};
-
 static void fsl_dcu_drm_encoder_destroy(struct drm_encoder *encoder)
 {
 	drm_encoder_cleanup(encoder);
@@ -68,13 +36,16 @@ int fsl_dcu_drm_encoder_create(struct fsl_dcu_drm_device *fsl_dev,
 	int ret;
 
 	encoder->possible_crtcs = 1;
+
+	/* Use bypass mode for parallel RGB/LVDS encoder */
+	if (fsl_dev->tcon)
+		fsl_tcon_bypass_enable(fsl_dev->tcon);
+
 	ret = drm_encoder_init(fsl_dev->drm, encoder, &encoder_funcs,
 			       DRM_MODE_ENCODER_LVDS, NULL);
 	if (ret < 0)
 		return ret;
 
-	drm_encoder_helper_add(encoder, &encoder_helper_funcs);
-
 	return 0;
 }
 
-- 
2.10.0

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 2/5] drm/fsl-dcu: do not transfer registers on plane init
  2016-10-17 21:33 ` Stefan Agner
@ 2016-10-17 21:33   ` Stefan Agner
  -1 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel
  Cc: alison.wang, jianwei.wang.chn, linux-kernel, Stefan Agner

There is no need to explicitly initiate a register transfer and
turn off the DCU after initializing the plane registers. In fact,
this is harmful and leads to unnecessary flickers if the DCU has
been left on by the bootloader.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index a7e5486..9e6f7d8 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -211,11 +211,6 @@ void fsl_dcu_drm_init_planes(struct drm_device *dev)
 		for (j = 1; j <= fsl_dev->soc->layer_regs; j++)
 			regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(i, j), 0);
 	}
-	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
-			   DCU_MODE_DCU_MODE_MASK,
-			   DCU_MODE_DCU_MODE(DCU_MODE_OFF));
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 }
 
 struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 2/5] drm/fsl-dcu: do not transfer registers on plane init
@ 2016-10-17 21:33   ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel; +Cc: jianwei.wang.chn, linux-kernel, alison.wang

There is no need to explicitly initiate a register transfer and
turn off the DCU after initializing the plane registers. In fact,
this is harmful and leads to unnecessary flickers if the DCU has
been left on by the bootloader.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index a7e5486..9e6f7d8 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -211,11 +211,6 @@ void fsl_dcu_drm_init_planes(struct drm_device *dev)
 		for (j = 1; j <= fsl_dev->soc->layer_regs; j++)
 			regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(i, j), 0);
 	}
-	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
-			   DCU_MODE_DCU_MODE_MASK,
-			   DCU_MODE_DCU_MODE(DCU_MODE_OFF));
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 }
 
 struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
-- 
2.10.0

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 3/5] drm/fsl-dcu: do not transfer registers in mode_set_nofb
  2016-10-17 21:33 ` Stefan Agner
@ 2016-10-17 21:33   ` Stefan Agner
  -1 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel
  Cc: alison.wang, jianwei.wang.chn, linux-kernel, Stefan Agner

Do not schedule a transfer of mode settings early. Modes should
get applied on on CRTC enable where we also enable the pixel clock.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 3371635..5ad1d68 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -116,8 +116,6 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 		     DCU_THRESHOLD_LS_BF_VS(BF_VS_VAL) |
 		     DCU_THRESHOLD_OUT_BUF_HIGH(BUF_MAX_VAL) |
 		     DCU_THRESHOLD_OUT_BUF_LOW(BUF_MIN_VAL));
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 	return;
 }
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 3/5] drm/fsl-dcu: do not transfer registers in mode_set_nofb
@ 2016-10-17 21:33   ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel; +Cc: jianwei.wang.chn, linux-kernel, alison.wang

Do not schedule a transfer of mode settings early. Modes should
get applied on on CRTC enable where we also enable the pixel clock.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 3371635..5ad1d68 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -116,8 +116,6 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 		     DCU_THRESHOLD_LS_BF_VS(BF_VS_VAL) |
 		     DCU_THRESHOLD_OUT_BUF_HIGH(BUF_MAX_VAL) |
 		     DCU_THRESHOLD_OUT_BUF_LOW(BUF_MIN_VAL));
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
 	return;
 }
 
-- 
2.10.0

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 4/5] drm/fsl-dcu: enable pixel clock when enabling CRTC
  2016-10-17 21:33 ` Stefan Agner
@ 2016-10-17 21:33   ` Stefan Agner
  -1 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel
  Cc: alison.wang, jianwei.wang.chn, linux-kernel, Stefan Agner

The pixel clock should not be on if the CRTC is not in use, hence
move clock enable/disable calls into CRTC callbacks.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c |  2 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c  | 21 +--------------------
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 5ad1d68..b2d5e18 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -51,6 +51,7 @@ static void fsl_dcu_drm_disable_crtc(struct drm_crtc *crtc)
 			   DCU_MODE_DCU_MODE(DCU_MODE_OFF));
 	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
 		     DCU_UPDATE_MODE_READREG);
+	clk_disable_unprepare(fsl_dev->pix_clk);
 }
 
 static void fsl_dcu_drm_crtc_enable(struct drm_crtc *crtc)
@@ -58,6 +59,7 @@ static void fsl_dcu_drm_crtc_enable(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
 
+	clk_prepare_enable(fsl_dev->pix_clk);
 	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
 			   DCU_MODE_DCU_MODE_MASK,
 			   DCU_MODE_DCU_MODE(DCU_MODE_NORMAL));
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 3897f56..e04efbe 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -267,12 +267,6 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 		return ret;
 	}
 
-	ret = clk_prepare_enable(fsl_dev->pix_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable pix clk\n");
-		goto disable_dcu_clk;
-	}
-
 	if (fsl_dev->tcon)
 		fsl_tcon_bypass_enable(fsl_dev->tcon);
 	fsl_dcu_drm_init_planes(fsl_dev->drm);
@@ -286,10 +280,6 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 	enable_irq(fsl_dev->irq);
 
 	return 0;
-
-disable_dcu_clk:
-	clk_disable_unprepare(fsl_dev->clk);
-	return ret;
 }
 #endif
 
@@ -403,18 +393,12 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
-	ret = clk_prepare_enable(fsl_dev->pix_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable pix clk\n");
-		goto unregister_pix_clk;
-	}
-
 	fsl_dev->tcon = fsl_tcon_init(dev);
 
 	drm = drm_dev_alloc(driver, dev);
 	if (IS_ERR(drm)) {
 		ret = PTR_ERR(drm);
-		goto disable_pix_clk;
+		goto unregister_pix_clk;
 	}
 
 	fsl_dev->dev = dev;
@@ -435,8 +419,6 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
 
 unref:
 	drm_dev_unref(drm);
-disable_pix_clk:
-	clk_disable_unprepare(fsl_dev->pix_clk);
 unregister_pix_clk:
 	clk_unregister(fsl_dev->pix_clk);
 disable_clk:
@@ -449,7 +431,6 @@ static int fsl_dcu_drm_remove(struct platform_device *pdev)
 	struct fsl_dcu_drm_device *fsl_dev = platform_get_drvdata(pdev);
 
 	clk_disable_unprepare(fsl_dev->clk);
-	clk_disable_unprepare(fsl_dev->pix_clk);
 	clk_unregister(fsl_dev->pix_clk);
 	drm_put_dev(fsl_dev->drm);
 
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 4/5] drm/fsl-dcu: enable pixel clock when enabling CRTC
@ 2016-10-17 21:33   ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel; +Cc: jianwei.wang.chn, linux-kernel, alison.wang

The pixel clock should not be on if the CRTC is not in use, hence
move clock enable/disable calls into CRTC callbacks.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c |  2 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c  | 21 +--------------------
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 5ad1d68..b2d5e18 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -51,6 +51,7 @@ static void fsl_dcu_drm_disable_crtc(struct drm_crtc *crtc)
 			   DCU_MODE_DCU_MODE(DCU_MODE_OFF));
 	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
 		     DCU_UPDATE_MODE_READREG);
+	clk_disable_unprepare(fsl_dev->pix_clk);
 }
 
 static void fsl_dcu_drm_crtc_enable(struct drm_crtc *crtc)
@@ -58,6 +59,7 @@ static void fsl_dcu_drm_crtc_enable(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
 
+	clk_prepare_enable(fsl_dev->pix_clk);
 	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
 			   DCU_MODE_DCU_MODE_MASK,
 			   DCU_MODE_DCU_MODE(DCU_MODE_NORMAL));
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 3897f56..e04efbe 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -267,12 +267,6 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 		return ret;
 	}
 
-	ret = clk_prepare_enable(fsl_dev->pix_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable pix clk\n");
-		goto disable_dcu_clk;
-	}
-
 	if (fsl_dev->tcon)
 		fsl_tcon_bypass_enable(fsl_dev->tcon);
 	fsl_dcu_drm_init_planes(fsl_dev->drm);
@@ -286,10 +280,6 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 	enable_irq(fsl_dev->irq);
 
 	return 0;
-
-disable_dcu_clk:
-	clk_disable_unprepare(fsl_dev->clk);
-	return ret;
 }
 #endif
 
@@ -403,18 +393,12 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
-	ret = clk_prepare_enable(fsl_dev->pix_clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable pix clk\n");
-		goto unregister_pix_clk;
-	}
-
 	fsl_dev->tcon = fsl_tcon_init(dev);
 
 	drm = drm_dev_alloc(driver, dev);
 	if (IS_ERR(drm)) {
 		ret = PTR_ERR(drm);
-		goto disable_pix_clk;
+		goto unregister_pix_clk;
 	}
 
 	fsl_dev->dev = dev;
@@ -435,8 +419,6 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
 
 unref:
 	drm_dev_unref(drm);
-disable_pix_clk:
-	clk_disable_unprepare(fsl_dev->pix_clk);
 unregister_pix_clk:
 	clk_unregister(fsl_dev->pix_clk);
 disable_clk:
@@ -449,7 +431,6 @@ static int fsl_dcu_drm_remove(struct platform_device *pdev)
 	struct fsl_dcu_drm_device *fsl_dev = platform_get_drvdata(pdev);
 
 	clk_disable_unprepare(fsl_dev->clk);
-	clk_disable_unprepare(fsl_dev->pix_clk);
 	clk_unregister(fsl_dev->pix_clk);
 	drm_put_dev(fsl_dev->drm);
 
-- 
2.10.0

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
  2016-10-17 21:33 ` Stefan Agner
@ 2016-10-17 21:33   ` Stefan Agner
  -1 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel
  Cc: alison.wang, jianwei.wang.chn, linux-kernel, Stefan Agner

There is no need to request a CMA backed framebuffer if fbdev
emulation is not enabled.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index e04efbe..3a5880c 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
 		goto done;
 	dev->irq_enabled = true;
 
-	fsl_dcu_fbdev_init(dev);
+	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
+		fsl_dcu_fbdev_init(dev);
 
 	return 0;
 done:
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
@ 2016-10-17 21:33   ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-17 21:33 UTC (permalink / raw)
  To: meng.yi, dri-devel; +Cc: jianwei.wang.chn, linux-kernel, alison.wang

There is no need to request a CMA backed framebuffer if fbdev
emulation is not enabled.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index e04efbe..3a5880c 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
 		goto done;
 	dev->irq_enabled = true;
 
-	fsl_dcu_fbdev_init(dev);
+	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
+		fsl_dcu_fbdev_init(dev);
 
 	return 0;
 done:
-- 
2.10.0

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
  2016-10-17 21:33   ` Stefan Agner
@ 2016-10-18  7:44     ` Daniel Vetter
  -1 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-10-18  7:44 UTC (permalink / raw)
  To: Stefan Agner
  Cc: meng.yi, dri-devel, jianwei.wang.chn, linux-kernel, alison.wang

On Mon, Oct 17, 2016 at 02:33:21PM -0700, Stefan Agner wrote:
> There is no need to request a CMA backed framebuffer if fbdev
> emulation is not enabled.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index e04efbe..3a5880c 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
>  		goto done;
>  	dev->irq_enabled = true;
>  
> -	fsl_dcu_fbdev_init(dev);
> +	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
> +		fsl_dcu_fbdev_init(dev);

Totally not required, since this will all no-op out. Also, please nuke
that fsl_dcu_fbdv_init wrapper seems like pointless indirection.

And if there really is an issue with the cma helpers allocating an fb when
they should, then the correct fix is to fix that in the helpers, not in
the drivers.

Nack.
-Daniel
>  
>  	return 0;
>  done:
> -- 
> 2.10.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
@ 2016-10-18  7:44     ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-10-18  7:44 UTC (permalink / raw)
  To: Stefan Agner
  Cc: jianwei.wang.chn, meng.yi, linux-kernel, dri-devel, alison.wang

On Mon, Oct 17, 2016 at 02:33:21PM -0700, Stefan Agner wrote:
> There is no need to request a CMA backed framebuffer if fbdev
> emulation is not enabled.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index e04efbe..3a5880c 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
>  		goto done;
>  	dev->irq_enabled = true;
>  
> -	fsl_dcu_fbdev_init(dev);
> +	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
> +		fsl_dcu_fbdev_init(dev);

Totally not required, since this will all no-op out. Also, please nuke
that fsl_dcu_fbdv_init wrapper seems like pointless indirection.

And if there really is an issue with the cma helpers allocating an fb when
they should, then the correct fix is to fix that in the helpers, not in
the drivers.

Nack.
-Daniel
>  
>  	return 0;
>  done:
> -- 
> 2.10.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
  2016-10-18  7:44     ` Daniel Vetter
@ 2016-10-18 17:42       ` Stefan Agner
  -1 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-18 17:42 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: meng.yi, dri-devel, jianwei.wang.chn, linux-kernel, alison.wang,
	Stefan Agner

On 2016-10-18 00:44, Daniel Vetter wrote:
> On Mon, Oct 17, 2016 at 02:33:21PM -0700, Stefan Agner wrote:
>> There is no need to request a CMA backed framebuffer if fbdev
>> emulation is not enabled.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> index e04efbe..3a5880c 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> @@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
>>  		goto done;
>>  	dev->irq_enabled = true;
>>
>> -	fsl_dcu_fbdev_init(dev);
>> +	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
>> +		fsl_dcu_fbdev_init(dev);
> 
> Totally not required, since this will all no-op out. Also, please nuke
> that fsl_dcu_fbdv_init wrapper seems like pointless indirection.

Regarding fsl_dcu_fbdev_init wrapper: Fully agreed. That thing was there
since inception of that driver, and I always was on the fence of doing
it. Will do it for the next merge window!

I somehow remembered there was something more to it than just "no need",
but I somehow failed to document it in the patch description... So I
went back and tested some things without the patch, here is when it
blows:

Unable to handle kernel NULL pointer dereference at virtual address
000002f0
pgd = cc24c000
[000002f0] *pgd=8c0df831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 536 Comm: sh Not tainted 4.9.0-rc1-00001-g4b1532a-dirty #568
Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
task: cc213000 task.stack: cc23e000
PC is at drm_fbdev_cma_fini+0x14/0x5c
LR is at fsl_dcu_unload+0x28/0x4c
pc : [<c04cfb20>]    lr : [<c04fad74>]    psr: a0000013
sp : cc23fd80  ip : cc23fd98  fp : cc23fd94
r10: cc1d1e4c  r9 : cc23e000  r8 : 00000000
r7 : c0e34888  r6 : 0000000d  r5 : 00000000  r4 : ce6ff100
r3 : c0e13b18  r2 : c0e13b18  r1 : 00000110  r0 : ce6ff100
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 8c24c04a  DAC: 00000051
Process sh (pid: 536, stack limit = 0xcc23e210)
Stack: (0xcc23fd80 to 0xcc240000)
...
Backtrace:
[<c04cfb0c>] (drm_fbdev_cma_fini) from [<c04fad74>]
(fsl_dcu_unload+0x28/0x4c)
 r5:ce61e810[  372.213609]  r4:ce61f000

[<c04fad4c>] (fsl_dcu_unload) from [<c04d9550>]
(drm_dev_unregister+0x38/0xbc)
 r5:ce61f000[  372.228535]  r4:ce61f000
...

> 
> And if there really is an issue with the cma helpers allocating an fb when
> they should, then the correct fix is to fix that in the helpers, not in
> the drivers.

Hm, to safe memory, it would probably be best to do something like:

--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -492,6 +492,9 @@ struct drm_fbdev_cma
*drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
        struct drm_fb_helper *helper;
        int ret;
 
+       if (!drm_fbdev_emulation)
+               return NULL;
+
        fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
        if (!fbdev_cma) {
                dev_err(dev->dev, "Failed to allocate drm fbdev.\n");

But we don't have drm_fbdev_emulation emulation there.

Maybe just add some NULL check in drm_fbdev_cma_fini?

--
Stefan


> 
> Nack.
> -Daniel
>>
>>  	return 0;
>>  done:
>> --
>> 2.10.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
@ 2016-10-18 17:42       ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-18 17:42 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: jianwei.wang.chn, meng.yi, alison.wang, linux-kernel, dri-devel

On 2016-10-18 00:44, Daniel Vetter wrote:
> On Mon, Oct 17, 2016 at 02:33:21PM -0700, Stefan Agner wrote:
>> There is no need to request a CMA backed framebuffer if fbdev
>> emulation is not enabled.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> index e04efbe..3a5880c 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> @@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
>>  		goto done;
>>  	dev->irq_enabled = true;
>>
>> -	fsl_dcu_fbdev_init(dev);
>> +	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
>> +		fsl_dcu_fbdev_init(dev);
> 
> Totally not required, since this will all no-op out. Also, please nuke
> that fsl_dcu_fbdv_init wrapper seems like pointless indirection.

Regarding fsl_dcu_fbdev_init wrapper: Fully agreed. That thing was there
since inception of that driver, and I always was on the fence of doing
it. Will do it for the next merge window!

I somehow remembered there was something more to it than just "no need",
but I somehow failed to document it in the patch description... So I
went back and tested some things without the patch, here is when it
blows:

Unable to handle kernel NULL pointer dereference at virtual address
000002f0
pgd = cc24c000
[000002f0] *pgd=8c0df831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 536 Comm: sh Not tainted 4.9.0-rc1-00001-g4b1532a-dirty #568
Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
task: cc213000 task.stack: cc23e000
PC is at drm_fbdev_cma_fini+0x14/0x5c
LR is at fsl_dcu_unload+0x28/0x4c
pc : [<c04cfb20>]    lr : [<c04fad74>]    psr: a0000013
sp : cc23fd80  ip : cc23fd98  fp : cc23fd94
r10: cc1d1e4c  r9 : cc23e000  r8 : 00000000
r7 : c0e34888  r6 : 0000000d  r5 : 00000000  r4 : ce6ff100
r3 : c0e13b18  r2 : c0e13b18  r1 : 00000110  r0 : ce6ff100
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 8c24c04a  DAC: 00000051
Process sh (pid: 536, stack limit = 0xcc23e210)
Stack: (0xcc23fd80 to 0xcc240000)
...
Backtrace:
[<c04cfb0c>] (drm_fbdev_cma_fini) from [<c04fad74>]
(fsl_dcu_unload+0x28/0x4c)
 r5:ce61e810[  372.213609]  r4:ce61f000

[<c04fad4c>] (fsl_dcu_unload) from [<c04d9550>]
(drm_dev_unregister+0x38/0xbc)
 r5:ce61f000[  372.228535]  r4:ce61f000
...

> 
> And if there really is an issue with the cma helpers allocating an fb when
> they should, then the correct fix is to fix that in the helpers, not in
> the drivers.

Hm, to safe memory, it would probably be best to do something like:

--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -492,6 +492,9 @@ struct drm_fbdev_cma
*drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
        struct drm_fb_helper *helper;
        int ret;
 
+       if (!drm_fbdev_emulation)
+               return NULL;
+
        fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
        if (!fbdev_cma) {
                dev_err(dev->dev, "Failed to allocate drm fbdev.\n");

But we don't have drm_fbdev_emulation emulation there.

Maybe just add some NULL check in drm_fbdev_cma_fini?

--
Stefan


> 
> Nack.
> -Daniel
>>
>>  	return 0;
>>  done:
>> --
>> 2.10.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
  2016-10-18 17:42       ` Stefan Agner
@ 2016-10-19  7:42         ` Daniel Vetter
  -1 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-10-19  7:42 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Daniel Vetter, meng.yi, dri-devel, jianwei.wang.chn,
	linux-kernel, alison.wang

On Tue, Oct 18, 2016 at 10:42:46AM -0700, Stefan Agner wrote:
> On 2016-10-18 00:44, Daniel Vetter wrote:
> > On Mon, Oct 17, 2016 at 02:33:21PM -0700, Stefan Agner wrote:
> >> There is no need to request a CMA backed framebuffer if fbdev
> >> emulation is not enabled.
> >>
> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> index e04efbe..3a5880c 100644
> >> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> @@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
> >>  		goto done;
> >>  	dev->irq_enabled = true;
> >>
> >> -	fsl_dcu_fbdev_init(dev);
> >> +	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
> >> +		fsl_dcu_fbdev_init(dev);
> > 
> > Totally not required, since this will all no-op out. Also, please nuke
> > that fsl_dcu_fbdv_init wrapper seems like pointless indirection.
> 
> Regarding fsl_dcu_fbdev_init wrapper: Fully agreed. That thing was there
> since inception of that driver, and I always was on the fence of doing
> it. Will do it for the next merge window!
> 
> I somehow remembered there was something more to it than just "no need",
> but I somehow failed to document it in the patch description... So I
> went back and tested some things without the patch, here is when it
> blows:
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 000002f0
> pgd = cc24c000
> [000002f0] *pgd=8c0df831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 17 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 536 Comm: sh Not tainted 4.9.0-rc1-00001-g4b1532a-dirty #568
> Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
> task: cc213000 task.stack: cc23e000
> PC is at drm_fbdev_cma_fini+0x14/0x5c
> LR is at fsl_dcu_unload+0x28/0x4c
> pc : [<c04cfb20>]    lr : [<c04fad74>]    psr: a0000013
> sp : cc23fd80  ip : cc23fd98  fp : cc23fd94
> r10: cc1d1e4c  r9 : cc23e000  r8 : 00000000
> r7 : c0e34888  r6 : 0000000d  r5 : 00000000  r4 : ce6ff100
> r3 : c0e13b18  r2 : c0e13b18  r1 : 00000110  r0 : ce6ff100
> Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 8c24c04a  DAC: 00000051
> Process sh (pid: 536, stack limit = 0xcc23e210)
> Stack: (0xcc23fd80 to 0xcc240000)
> ...
> Backtrace:
> [<c04cfb0c>] (drm_fbdev_cma_fini) from [<c04fad74>]
> (fsl_dcu_unload+0x28/0x4c)
>  r5:ce61e810[  372.213609]  r4:ce61f000
> 
> [<c04fad4c>] (fsl_dcu_unload) from [<c04d9550>]
> (drm_dev_unregister+0x38/0xbc)
>  r5:ce61f000[  372.228535]  r4:ce61f000
> ...
> 
> > 
> > And if there really is an issue with the cma helpers allocating an fb when
> > they should, then the correct fix is to fix that in the helpers, not in
> > the drivers.
> 
> Hm, to safe memory, it would probably be best to do something like:
> 
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -492,6 +492,9 @@ struct drm_fbdev_cma
> *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
>         struct drm_fb_helper *helper;
>         int ret;
>  
> +       if (!drm_fbdev_emulation)
> +               return NULL;
> +
>         fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
>         if (!fbdev_cma) {
>                 dev_err(dev->dev, "Failed to allocate drm fbdev.\n");
> 
> But we don't have drm_fbdev_emulation emulation there.

Not just that, but you'd leak memory since cma_init always does the
kmalloc of the fbdev_cma struct.

> Maybe just add some NULL check in drm_fbdev_cma_fini?

Probably. I can't read arm-oopses, so no idea what exactly blew up. On a
hunch it's probably drm_fbdev_cma_defio_fini that got things wrong and
needs to check for !fbi || !fbi->defio.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required
@ 2016-10-19  7:42         ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2016-10-19  7:42 UTC (permalink / raw)
  To: Stefan Agner
  Cc: jianwei.wang.chn, meng.yi, alison.wang, Daniel Vetter,
	linux-kernel, dri-devel

On Tue, Oct 18, 2016 at 10:42:46AM -0700, Stefan Agner wrote:
> On 2016-10-18 00:44, Daniel Vetter wrote:
> > On Mon, Oct 17, 2016 at 02:33:21PM -0700, Stefan Agner wrote:
> >> There is no need to request a CMA backed framebuffer if fbdev
> >> emulation is not enabled.
> >>
> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> index e04efbe..3a5880c 100644
> >> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> >> @@ -87,7 +87,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
> >>  		goto done;
> >>  	dev->irq_enabled = true;
> >>
> >> -	fsl_dcu_fbdev_init(dev);
> >> +	if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION))
> >> +		fsl_dcu_fbdev_init(dev);
> > 
> > Totally not required, since this will all no-op out. Also, please nuke
> > that fsl_dcu_fbdv_init wrapper seems like pointless indirection.
> 
> Regarding fsl_dcu_fbdev_init wrapper: Fully agreed. That thing was there
> since inception of that driver, and I always was on the fence of doing
> it. Will do it for the next merge window!
> 
> I somehow remembered there was something more to it than just "no need",
> but I somehow failed to document it in the patch description... So I
> went back and tested some things without the patch, here is when it
> blows:
> 
> Unable to handle kernel NULL pointer dereference at virtual address
> 000002f0
> pgd = cc24c000
> [000002f0] *pgd=8c0df831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 17 [#1] SMP ARM
> Modules linked in:
> CPU: 0 PID: 536 Comm: sh Not tainted 4.9.0-rc1-00001-g4b1532a-dirty #568
> Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
> task: cc213000 task.stack: cc23e000
> PC is at drm_fbdev_cma_fini+0x14/0x5c
> LR is at fsl_dcu_unload+0x28/0x4c
> pc : [<c04cfb20>]    lr : [<c04fad74>]    psr: a0000013
> sp : cc23fd80  ip : cc23fd98  fp : cc23fd94
> r10: cc1d1e4c  r9 : cc23e000  r8 : 00000000
> r7 : c0e34888  r6 : 0000000d  r5 : 00000000  r4 : ce6ff100
> r3 : c0e13b18  r2 : c0e13b18  r1 : 00000110  r0 : ce6ff100
> Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 8c24c04a  DAC: 00000051
> Process sh (pid: 536, stack limit = 0xcc23e210)
> Stack: (0xcc23fd80 to 0xcc240000)
> ...
> Backtrace:
> [<c04cfb0c>] (drm_fbdev_cma_fini) from [<c04fad74>]
> (fsl_dcu_unload+0x28/0x4c)
>  r5:ce61e810[  372.213609]  r4:ce61f000
> 
> [<c04fad4c>] (fsl_dcu_unload) from [<c04d9550>]
> (drm_dev_unregister+0x38/0xbc)
>  r5:ce61f000[  372.228535]  r4:ce61f000
> ...
> 
> > 
> > And if there really is an issue with the cma helpers allocating an fb when
> > they should, then the correct fix is to fix that in the helpers, not in
> > the drivers.
> 
> Hm, to safe memory, it would probably be best to do something like:
> 
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -492,6 +492,9 @@ struct drm_fbdev_cma
> *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
>         struct drm_fb_helper *helper;
>         int ret;
>  
> +       if (!drm_fbdev_emulation)
> +               return NULL;
> +
>         fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
>         if (!fbdev_cma) {
>                 dev_err(dev->dev, "Failed to allocate drm fbdev.\n");
> 
> But we don't have drm_fbdev_emulation emulation there.

Not just that, but you'd leak memory since cma_init always does the
kmalloc of the fbdev_cma struct.

> Maybe just add some NULL check in drm_fbdev_cma_fini?

Probably. I can't read arm-oopses, so no idea what exactly blew up. On a
hunch it's probably drm_fbdev_cma_defio_fini that got things wrong and
needs to check for !fbi || !fbi->defio.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread
* RE: [PATCH v3 1/5] drm/fsl-dcu: enable TCON bypass mode by default
  2016-10-17 21:33   ` Stefan Agner
  (?)
@ 2016-10-19  9:35   ` Meng Yi
  2016-10-20  0:01       ` Stefan Agner
  -1 siblings, 1 reply; 21+ messages in thread
From: Meng Yi @ 2016-10-19  9:35 UTC (permalink / raw)
  To: Stefan Agner, dri-devel; +Cc: alison.wang, jianwei.wang.chn, linux-kernel


> 
> Do not use encoder disable/enable callbacks to control bypass mode as this
> seems to mess with the signals not liked by displays. This also makes more
> sense since the encoder is already defined to be parallel RGB/LVDS at creation
> time.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c |  2 ++  drivers/gpu/drm/fsl-
> dcu/fsl_dcu_drm_rgb.c | 39 ++++---------------------------
>  2 files changed, 7 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-
> dcu/fsl_dcu_drm_drv.c
> index 0884c45..3897f56 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -273,6 +273,8 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
>  		goto disable_dcu_clk;
>  	}
> 
> +	if (fsl_dev->tcon)
> +		fsl_tcon_bypass_enable(fsl_dev->tcon);
>  	fsl_dcu_drm_init_planes(fsl_dev->drm);
>  	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-
> dcu/fsl_dcu_drm_rgb.c
> index 26edcc8..e1dd75b 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
> @@ -20,38 +20,6 @@
>  #include "fsl_dcu_drm_drv.h"
>  #include "fsl_tcon.h"
> 
> -static int
> -fsl_dcu_drm_encoder_atomic_check(struct drm_encoder *encoder,
> -				 struct drm_crtc_state *crtc_state,
> -				 struct drm_connector_state *conn_state)
> -{
> -	return 0;
> -}
> -
> -static void fsl_dcu_drm_encoder_disable(struct drm_encoder *encoder) -{
> -	struct drm_device *dev = encoder->dev;
> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> -
> -	if (fsl_dev->tcon)
> -		fsl_tcon_bypass_disable(fsl_dev->tcon);
> -}
> -
> -static void fsl_dcu_drm_encoder_enable(struct drm_encoder *encoder) -{
> -	struct drm_device *dev = encoder->dev;
> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> -
> -	if (fsl_dev->tcon)
> -		fsl_tcon_bypass_enable(fsl_dev->tcon);
> -}
> -
> -static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
> -	.atomic_check = fsl_dcu_drm_encoder_atomic_check,
> -	.disable = fsl_dcu_drm_encoder_disable,
> -	.enable = fsl_dcu_drm_encoder_enable,
> -};
> -
>  static void fsl_dcu_drm_encoder_destroy(struct drm_encoder *encoder)  {
>  	drm_encoder_cleanup(encoder);
> @@ -68,13 +36,16 @@ int fsl_dcu_drm_encoder_create(struct
> fsl_dcu_drm_device *fsl_dev,
>  	int ret;
> 
>  	encoder->possible_crtcs = 1;
> +
> +	/* Use bypass mode for parallel RGB/LVDS encoder */
> +	if (fsl_dev->tcon)
> +		fsl_tcon_bypass_enable(fsl_dev->tcon);
> +
>  	ret = drm_encoder_init(fsl_dev->drm, encoder, &encoder_funcs,
>  			       DRM_MODE_ENCODER_LVDS, NULL);
>  	if (ret < 0)
>  		return ret;
> 
> -	drm_encoder_helper_add(encoder, &encoder_helper_funcs);
> -
>  	return 0;
>  }
> 
> --
> 2.10.0

Tested-By: Meng Yi <meng.yi@nxp.com>

Tested those 5 patches on LS1021a-twr, and everything is fine.

Meng

^ permalink raw reply	[flat|nested] 21+ messages in thread
* RE: [PATCH v3 1/5] drm/fsl-dcu: enable TCON bypass mode by default
  2016-10-19  9:35   ` Meng Yi
@ 2016-10-20  0:01       ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-20  0:01 UTC (permalink / raw)
  To: Meng Yi; +Cc: dri-devel, alison.wang, jianwei.wang.chn, linux-kernel

On 2016-10-19 02:35, Meng Yi wrote:
>>
>> Do not use encoder disable/enable callbacks to control bypass mode as this
>> seems to mess with the signals not liked by displays. This also makes more
>> sense since the encoder is already defined to be parallel RGB/LVDS at creation
>> time.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c |  2 ++  drivers/gpu/drm/fsl-
>> dcu/fsl_dcu_drm_rgb.c | 39 ++++---------------------------
>>  2 files changed, 7 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-
>> dcu/fsl_dcu_drm_drv.c
>> index 0884c45..3897f56 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> @@ -273,6 +273,8 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
>>  		goto disable_dcu_clk;
>>  	}
>>
>> +	if (fsl_dev->tcon)
>> +		fsl_tcon_bypass_enable(fsl_dev->tcon);
>>  	fsl_dcu_drm_init_planes(fsl_dev->drm);
>>  	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-
>> dcu/fsl_dcu_drm_rgb.c
>> index 26edcc8..e1dd75b 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
>> @@ -20,38 +20,6 @@
>>  #include "fsl_dcu_drm_drv.h"
>>  #include "fsl_tcon.h"
>>
>> -static int
>> -fsl_dcu_drm_encoder_atomic_check(struct drm_encoder *encoder,
>> -				 struct drm_crtc_state *crtc_state,
>> -				 struct drm_connector_state *conn_state)
>> -{
>> -	return 0;
>> -}
>> -
>> -static void fsl_dcu_drm_encoder_disable(struct drm_encoder *encoder) -{
>> -	struct drm_device *dev = encoder->dev;
>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>> -
>> -	if (fsl_dev->tcon)
>> -		fsl_tcon_bypass_disable(fsl_dev->tcon);
>> -}
>> -
>> -static void fsl_dcu_drm_encoder_enable(struct drm_encoder *encoder) -{
>> -	struct drm_device *dev = encoder->dev;
>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>> -
>> -	if (fsl_dev->tcon)
>> -		fsl_tcon_bypass_enable(fsl_dev->tcon);
>> -}
>> -
>> -static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
>> -	.atomic_check = fsl_dcu_drm_encoder_atomic_check,
>> -	.disable = fsl_dcu_drm_encoder_disable,
>> -	.enable = fsl_dcu_drm_encoder_enable,
>> -};
>> -
>>  static void fsl_dcu_drm_encoder_destroy(struct drm_encoder *encoder)  {
>>  	drm_encoder_cleanup(encoder);
>> @@ -68,13 +36,16 @@ int fsl_dcu_drm_encoder_create(struct
>> fsl_dcu_drm_device *fsl_dev,
>>  	int ret;
>>
>>  	encoder->possible_crtcs = 1;
>> +
>> +	/* Use bypass mode for parallel RGB/LVDS encoder */
>> +	if (fsl_dev->tcon)
>> +		fsl_tcon_bypass_enable(fsl_dev->tcon);
>> +
>>  	ret = drm_encoder_init(fsl_dev->drm, encoder, &encoder_funcs,
>>  			       DRM_MODE_ENCODER_LVDS, NULL);
>>  	if (ret < 0)
>>  		return ret;
>>
>> -	drm_encoder_helper_add(encoder, &encoder_helper_funcs);
>> -
>>  	return 0;
>>  }
>>
>> --
>> 2.10.0
> 
> Tested-By: Meng Yi <meng.yi@nxp.com>
> 
> Tested those 5 patches on LS1021a-twr, and everything is fine.

Thanks.

Applied 1~4.

--
Stefan

^ permalink raw reply	[flat|nested] 21+ messages in thread
* RE: [PATCH v3 1/5] drm/fsl-dcu: enable TCON bypass mode by default
@ 2016-10-20  0:01       ` Stefan Agner
  0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2016-10-20  0:01 UTC (permalink / raw)
  To: Meng Yi; +Cc: jianwei.wang.chn, linux-kernel, dri-devel, alison.wang

On 2016-10-19 02:35, Meng Yi wrote:
>>
>> Do not use encoder disable/enable callbacks to control bypass mode as this
>> seems to mess with the signals not liked by displays. This also makes more
>> sense since the encoder is already defined to be parallel RGB/LVDS at creation
>> time.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c |  2 ++  drivers/gpu/drm/fsl-
>> dcu/fsl_dcu_drm_rgb.c | 39 ++++---------------------------
>>  2 files changed, 7 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-
>> dcu/fsl_dcu_drm_drv.c
>> index 0884c45..3897f56 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> @@ -273,6 +273,8 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
>>  		goto disable_dcu_clk;
>>  	}
>>
>> +	if (fsl_dev->tcon)
>> +		fsl_tcon_bypass_enable(fsl_dev->tcon);
>>  	fsl_dcu_drm_init_planes(fsl_dev->drm);
>>  	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-
>> dcu/fsl_dcu_drm_rgb.c
>> index 26edcc8..e1dd75b 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
>> @@ -20,38 +20,6 @@
>>  #include "fsl_dcu_drm_drv.h"
>>  #include "fsl_tcon.h"
>>
>> -static int
>> -fsl_dcu_drm_encoder_atomic_check(struct drm_encoder *encoder,
>> -				 struct drm_crtc_state *crtc_state,
>> -				 struct drm_connector_state *conn_state)
>> -{
>> -	return 0;
>> -}
>> -
>> -static void fsl_dcu_drm_encoder_disable(struct drm_encoder *encoder) -{
>> -	struct drm_device *dev = encoder->dev;
>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>> -
>> -	if (fsl_dev->tcon)
>> -		fsl_tcon_bypass_disable(fsl_dev->tcon);
>> -}
>> -
>> -static void fsl_dcu_drm_encoder_enable(struct drm_encoder *encoder) -{
>> -	struct drm_device *dev = encoder->dev;
>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>> -
>> -	if (fsl_dev->tcon)
>> -		fsl_tcon_bypass_enable(fsl_dev->tcon);
>> -}
>> -
>> -static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
>> -	.atomic_check = fsl_dcu_drm_encoder_atomic_check,
>> -	.disable = fsl_dcu_drm_encoder_disable,
>> -	.enable = fsl_dcu_drm_encoder_enable,
>> -};
>> -
>>  static void fsl_dcu_drm_encoder_destroy(struct drm_encoder *encoder)  {
>>  	drm_encoder_cleanup(encoder);
>> @@ -68,13 +36,16 @@ int fsl_dcu_drm_encoder_create(struct
>> fsl_dcu_drm_device *fsl_dev,
>>  	int ret;
>>
>>  	encoder->possible_crtcs = 1;
>> +
>> +	/* Use bypass mode for parallel RGB/LVDS encoder */
>> +	if (fsl_dev->tcon)
>> +		fsl_tcon_bypass_enable(fsl_dev->tcon);
>> +
>>  	ret = drm_encoder_init(fsl_dev->drm, encoder, &encoder_funcs,
>>  			       DRM_MODE_ENCODER_LVDS, NULL);
>>  	if (ret < 0)
>>  		return ret;
>>
>> -	drm_encoder_helper_add(encoder, &encoder_helper_funcs);
>> -
>>  	return 0;
>>  }
>>
>> --
>> 2.10.0
> 
> Tested-By: Meng Yi <meng.yi@nxp.com>
> 
> Tested those 5 patches on LS1021a-twr, and everything is fine.

Thanks.

Applied 1~4.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-10-20  0:07 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-17 21:33 [PATCH v3 0/5] drm/fsl-dcu: initialization fixes Stefan Agner
2016-10-17 21:33 ` Stefan Agner
2016-10-17 21:33 ` [PATCH v3 1/5] drm/fsl-dcu: enable TCON bypass mode by default Stefan Agner
2016-10-17 21:33   ` Stefan Agner
2016-10-19  9:35   ` Meng Yi
2016-10-20  0:01     ` Stefan Agner
2016-10-20  0:01       ` Stefan Agner
2016-10-17 21:33 ` [PATCH v3 2/5] drm/fsl-dcu: do not transfer registers on plane init Stefan Agner
2016-10-17 21:33   ` Stefan Agner
2016-10-17 21:33 ` [PATCH v3 3/5] drm/fsl-dcu: do not transfer registers in mode_set_nofb Stefan Agner
2016-10-17 21:33   ` Stefan Agner
2016-10-17 21:33 ` [PATCH v3 4/5] drm/fsl-dcu: enable pixel clock when enabling CRTC Stefan Agner
2016-10-17 21:33   ` Stefan Agner
2016-10-17 21:33 ` [PATCH v3 5/5] drm/fsl-dcu: only init fbdev if required Stefan Agner
2016-10-17 21:33   ` Stefan Agner
2016-10-18  7:44   ` Daniel Vetter
2016-10-18  7:44     ` Daniel Vetter
2016-10-18 17:42     ` Stefan Agner
2016-10-18 17:42       ` Stefan Agner
2016-10-19  7:42       ` Daniel Vetter
2016-10-19  7:42         ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.