diff mbox series

[v2,2/2] drm: fsl-dcu: enable PIXCLK on LS1021A

Message ID 20240926055552.1632448-2-alexander.stein@ew.tq-group.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] drm: fsl-dcu: Use dev_err_probe | expand

Commit Message

Alexander Stein Sept. 26, 2024, 5:55 a.m. UTC 
From: Matthias Schiffer <matthias.schiffer@tq-group.com>

The PIXCLK needs to be enabled in SCFG before accessing certain DCU
registers, or the access will hang. For simplicity, the PIXCLK is enabled
unconditionally, resulting in increased power consumption.

Signed-off-by: Matthias Schiffer <matthias.schiffer@tq-group.com>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Changes in v2:
* Add note about power consumption in commit message
* Add note about power consumption in comment
* Fix alignment

 drivers/gpu/drm/fsl-dcu/Kconfig           |  1 +
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 15 +++++++++++++++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  3 +++
 3 files changed, 19 insertions(+)

Comments

Dmitry Baryshkov Sept. 26, 2024, 6:05 a.m. UTC | #1 
On Thu, Sep 26, 2024 at 07:55:51AM GMT, Alexander Stein wrote:
> From: Matthias Schiffer <matthias.schiffer@tq-group.com>
> 
> The PIXCLK needs to be enabled in SCFG before accessing certain DCU
> registers, or the access will hang. For simplicity, the PIXCLK is enabled
> unconditionally, resulting in increased power consumption.

By this description it looks like a fix. What is the first broken
commit? It needs to be mentioned in the Fixes: tag. Or is it hat
existing devices have been enabling SCFG in some other way?

> 
> Signed-off-by: Matthias Schiffer <matthias.schiffer@tq-group.com>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Changes in v2:
> * Add note about power consumption in commit message
> * Add note about power consumption in comment
> * Fix alignment
>
Alexander Stein Sept. 26, 2024, 2:09 p.m. UTC | #2 
Hi Dmitry,

Am Donnerstag, 26. September 2024, 08:05:56 CEST schrieb Dmitry Baryshkov:
> On Thu, Sep 26, 2024 at 07:55:51AM GMT, Alexander Stein wrote:
> > From: Matthias Schiffer <matthias.schiffer@tq-group.com>
> > 
> > The PIXCLK needs to be enabled in SCFG before accessing certain DCU
> > registers, or the access will hang. For simplicity, the PIXCLK is enabled
> > unconditionally, resulting in increased power consumption.
> 
> By this description it looks like a fix. What is the first broken
> commit? It needs to be mentioned in the Fixes: tag. Or is it hat
> existing devices have been enabling SCFG in some other way?

We discussed this internally and it seems this never worked, unless PIXCLK
was already enabled in SCFG by a different way, e.g. firmware, etc.

Best regards,
Alexander

> > 
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@tq-group.com>
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Changes in v2:
> > * Add note about power consumption in commit message
> > * Add note about power consumption in comment
> > * Fix alignment
> > 
> 
>
Dmitry Baryshkov Sept. 26, 2024, 11:13 p.m. UTC | #3 
On Thu, Sep 26, 2024 at 04:09:03PM GMT, Alexander Stein wrote:
> Hi Dmitry,
> 
> Am Donnerstag, 26. September 2024, 08:05:56 CEST schrieb Dmitry Baryshkov:
> > On Thu, Sep 26, 2024 at 07:55:51AM GMT, Alexander Stein wrote:
> > > From: Matthias Schiffer <matthias.schiffer@tq-group.com>
> > > 
> > > The PIXCLK needs to be enabled in SCFG before accessing certain DCU
> > > registers, or the access will hang. For simplicity, the PIXCLK is enabled
> > > unconditionally, resulting in increased power consumption.
> > 
> > By this description it looks like a fix. What is the first broken
> > commit? It needs to be mentioned in the Fixes: tag. Or is it hat
> > existing devices have been enabling SCFG in some other way?
> 
> We discussed this internally and it seems this never worked, unless PIXCLK
> was already enabled in SCFG by a different way, e.g. firmware, etc.

My bet was on the firmware, but I never touched Layerscape platforms.
Anyway,

Fixes: 109eee2f2a18 ("drm/layerscape: Add Freescale DCU DRM driver")
Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> 
> Best regards,
> Alexander
> 
> > > 
> > > Signed-off-by: Matthias Schiffer <matthias.schiffer@tq-group.com>
> > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > ---
> > > Changes in v2:
> > > * Add note about power consumption in commit message
> > > * Add note about power consumption in comment
> > > * Fix alignment
> > > 
> > 
> > 
> 
> 
> -- 
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
> 
>
Alexander Stein Oct. 17, 2024, 6:50 a.m. UTC | #4 
Hi everyone,

Am Freitag, 27. September 2024, 01:13:57 CEST schrieb Dmitry Baryshkov:
> On Thu, Sep 26, 2024 at 04:09:03PM GMT, Alexander Stein wrote:
> > Hi Dmitry,
> > 
> > Am Donnerstag, 26. September 2024, 08:05:56 CEST schrieb Dmitry Baryshkov:
> > > On Thu, Sep 26, 2024 at 07:55:51AM GMT, Alexander Stein wrote:
> > > > From: Matthias Schiffer <matthias.schiffer@tq-group.com>
> > > > 
> > > > The PIXCLK needs to be enabled in SCFG before accessing certain DCU
> > > > registers, or the access will hang. For simplicity, the PIXCLK is enabled
> > > > unconditionally, resulting in increased power consumption.
> > > 
> > > By this description it looks like a fix. What is the first broken
> > > commit? It needs to be mentioned in the Fixes: tag. Or is it hat
> > > existing devices have been enabling SCFG in some other way?
> > 
> > We discussed this internally and it seems this never worked, unless PIXCLK
> > was already enabled in SCFG by a different way, e.g. firmware, etc.
> 
> My bet was on the firmware, but I never touched Layerscape platforms.
> Anyway,
> 
> Fixes: 109eee2f2a18 ("drm/layerscape: Add Freescale DCU DRM driver")
> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Any additional feedback?

Best regards,
Alexander
Dmitry Baryshkov Oct. 19, 2024, 2:30 p.m. UTC | #5 
On Thu, Oct 17, 2024 at 08:50:43AM +0200, Alexander Stein wrote:
> Hi everyone,
> 
> Am Freitag, 27. September 2024, 01:13:57 CEST schrieb Dmitry Baryshkov:
> > On Thu, Sep 26, 2024 at 04:09:03PM GMT, Alexander Stein wrote:
> > > Hi Dmitry,
> > > 
> > > Am Donnerstag, 26. September 2024, 08:05:56 CEST schrieb Dmitry Baryshkov:
> > > > On Thu, Sep 26, 2024 at 07:55:51AM GMT, Alexander Stein wrote:
> > > > > From: Matthias Schiffer <matthias.schiffer@tq-group.com>
> > > > > 
> > > > > The PIXCLK needs to be enabled in SCFG before accessing certain DCU
> > > > > registers, or the access will hang. For simplicity, the PIXCLK is enabled
> > > > > unconditionally, resulting in increased power consumption.
> > > > 
> > > > By this description it looks like a fix. What is the first broken
> > > > commit? It needs to be mentioned in the Fixes: tag. Or is it hat
> > > > existing devices have been enabling SCFG in some other way?
> > > 
> > > We discussed this internally and it seems this never worked, unless PIXCLK
> > > was already enabled in SCFG by a different way, e.g. firmware, etc.
> > 
> > My bet was on the firmware, but I never touched Layerscape platforms.
> > Anyway,
> > 
> > Fixes: 109eee2f2a18 ("drm/layerscape: Add Freescale DCU DRM driver")
> > Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> Any additional feedback?

No response from Stefan and Alison for nearly a month...
diff mbox series

Patch

diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig b/drivers/gpu/drm/fsl-dcu/Kconfig
index 5ca71ef873259..c9ee98693b48a 100644
--- a/drivers/gpu/drm/fsl-dcu/Kconfig
+++ b/drivers/gpu/drm/fsl-dcu/Kconfig
@@ -8,6 +8,7 @@  config DRM_FSL_DCU
 	select DRM_PANEL
 	select REGMAP_MMIO
 	select VIDEOMODE_HELPERS
+	select MFD_SYSCON if SOC_LS1021A
 	help
 	  Choose this option if you have an Freescale DCU chipset.
 	  If M is selected the module will be called fsl-dcu-drm.
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 dd820f19482ad..63da55fe2eaf8 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -100,12 +100,27 @@  static void fsl_dcu_irq_uninstall(struct drm_device *dev)
 static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
 {
 	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+	struct regmap *scfg;
 	int ret;
 
 	ret = fsl_dcu_drm_modeset_init(fsl_dev);
 	if (ret < 0)
 		return dev_err_probe(dev->dev, ret, "failed to initialize mode setting\n");
 
+	scfg = syscon_regmap_lookup_by_compatible("fsl,ls1021a-scfg");
+	if (PTR_ERR(scfg) != -ENODEV) {
+		/*
+		 * For simplicity, enable the PIXCLK unconditionally,
+		 * resulting in increased power consumption. Disabling
+		 * the clock in PM or on unload could be implemented as
+		 * a future improvement.
+		 */
+		ret = regmap_update_bits(scfg, SCFG_PIXCLKCR, SCFG_PIXCLKCR_PXCEN,
+					 SCFG_PIXCLKCR_PXCEN);
+		if (ret < 0)
+			return dev_err_probe(dev->dev, ret, "failed to enable pixclk\n");
+	}
+
 	ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
 	if (ret < 0) {
 		dev_err(dev->dev, "failed to initialize vblank\n");
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index e2049a0e8a92a..566396013c04a 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -160,6 +160,9 @@ 
 #define FSL_DCU_ARGB4444		12
 #define FSL_DCU_YUV422			14
 
+#define SCFG_PIXCLKCR			0x28
+#define SCFG_PIXCLKCR_PXCEN		BIT(31)
+
 #define VF610_LAYER_REG_NUM		9
 #define LS1021A_LAYER_REG_NUM		10