All of lore.kernel.org
 help / color / mirror / Atom feed
From: Raphael GALLAIS-POU - foss <raphael.gallais-pou@foss.st.com>
To: Yannick FERTRE - foss <yannick.fertre@foss.st.com>,
	Philippe CORNU - foss <philippe.cornu@foss.st.com>,
	Benjamin Gaignard <benjamin.gaignard@linaro.org>,
	David Airlie <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre TORGUE - foss <alexandre.torgue@foss.st.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-stm32@st-md-mailman.stormreply.com" 
	<linux-stm32@st-md-mailman.stormreply.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: Yannick FERTRE <yannick.fertre@st.com>,
	Philippe CORNU <philippe.cornu@st.com>,
	Marek Vasut <marex@denx.de>,
	"Raphael GALLAIS-POU - foss" <raphael.gallais-pou@foss.st.com>,
	Raphael GALLAIS-POU <raphael.gallais-pou@st.com>
Subject: [PATCH] drm/stm: ltdc: improve pm_runtime to stop clocks
Date: Tue, 29 Jun 2021 11:58:19 +0000	[thread overview]
Message-ID: <20210629115709.16145-1-raphael.gallais-pou@foss.st.com> (raw)

Bugzilla ticket: https://intbugzilla.st.com/show_bug.cgi?id=60620
Gerrit patch: https://gerrit.st.com/c/mpu/oe/st/linux-stm32/+/208093/

In the LTDC driver, pm_runtime_get_sync was wrongly used and caused the
LTDC pixel clock to be systematically enabled in the clock summary.

After one simple use of the LTDC by activating and deactivating,
the clock summary results as below:

~# cat /sys/kernel/debug/clk/clk_summary | grep ltdc
        ltdc_px               1        1        0    29700000          0     0  50000         N
              ltdc            0        0        0   133250000          0     0  50000         N

By doing so, pm_runtime_get_sync only increments the clock counter when
the driver was in not active, displaying the right information when the
LTDC is not in use, resulting of the below clock summary after deactivation
of the LTDC.

~# cat /sys/kernel/debug/clk/clk_summary | grep ltdc
        ltdc_px               0        0        0    29700000          0     0  50000         N
              ltdc            0        0        0   133250000          0     0  50000         N

The clocks are activated either by the crtc_set_nofb function or
by the crtc_atomic_enable function. A check of pm_runtime activity must
be done before set clocks on. This check must also be done for others
functions which access registers.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 drivers/gpu/drm/stm/ltdc.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 08b71248044d..bf9d18023698 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -425,10 +425,17 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
 	struct drm_device *ddev = crtc->dev;
+	int ret;
 
 	DRM_DEBUG_DRIVER("\n");
 
-	pm_runtime_get_sync(ddev->dev);
+	if (!pm_runtime_active(ddev->dev)) {
+		ret = pm_runtime_get_sync(ddev->dev);
+		if (ret) {
+			DRM_ERROR("Failed to set mode, cannot get sync\n");
+			return;
+		}
+	}
 
 	/* Sets the background color value */
 	reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
@@ -783,6 +790,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	struct drm_plane_state *newstate = drm_atomic_get_new_plane_state(state,
 									  plane);
 	struct drm_framebuffer *fb = newstate->fb;
+	struct drm_device *ddev = plane->dev;
 	u32 lofs = plane->index * LAY_OFS;
 	u32 x0 = newstate->crtc_x;
 	u32 x1 = newstate->crtc_x + newstate->crtc_w - 1;
@@ -792,6 +800,11 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	u32 val, pitch_in_bytes, line_length, paddr, ahbp, avbp, bpcr;
 	enum ltdc_pix_fmt pf;
 
+	if (!pm_runtime_active(ddev->dev)) {
+		DRM_DEBUG_DRIVER("crtc not activated");
+		return;
+	}
+
 	if (!newstate->crtc || !fb) {
 		DRM_DEBUG_DRIVER("fb or crtc NULL");
 		return;
@@ -897,8 +910,14 @@ static void ltdc_plane_atomic_disable(struct drm_plane *plane,
 	struct drm_plane_state *oldstate = drm_atomic_get_old_plane_state(state,
 									  plane);
 	struct ltdc_device *ldev = plane_to_ltdc(plane);
+	struct drm_device *ddev = plane->dev;
 	u32 lofs = plane->index * LAY_OFS;
 
+	if (!pm_runtime_active(ddev->dev)) {
+		DRM_DEBUG_DRIVER("crtc already deactivated");
+		return;
+	}
+
 	/* disable layer */
 	reg_clear(ldev->regs, LTDC_L1CR + lofs, LXCR_LEN);
 
-- 
2.24.3

WARNING: multiple messages have this Message-ID (diff)
From: Raphael GALLAIS-POU - foss <raphael.gallais-pou@foss.st.com>
To: Yannick FERTRE - foss <yannick.fertre@foss.st.com>,
	Philippe CORNU - foss <philippe.cornu@foss.st.com>,
	Benjamin Gaignard <benjamin.gaignard@linaro.org>,
	David Airlie <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre TORGUE - foss <alexandre.torgue@foss.st.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: Yannick FERTRE <yannick.fertre@st.com>,
	Philippe CORNU <philippe.cornu@st.com>,
	Marek Vasut <marex@denx.de>,
	"Raphael GALLAIS-POU - foss" <raphael.gallais-pou@foss.st.com>,
	Raphael GALLAIS-POU <raphael.gallais-pou@st.com>
Subject: [PATCH] drm/stm: ltdc: improve pm_runtime to stop clocks
Date: Tue, 29 Jun 2021 11:58:19 +0000	[thread overview]
Message-ID: <20210629115709.16145-1-raphael.gallais-pou@foss.st.com> (raw)

Bugzilla ticket: https://intbugzilla.st.com/show_bug.cgi?id=60620
Gerrit patch: https://gerrit.st.com/c/mpu/oe/st/linux-stm32/+/208093/

In the LTDC driver, pm_runtime_get_sync was wrongly used and caused the
LTDC pixel clock to be systematically enabled in the clock summary.

After one simple use of the LTDC by activating and deactivating,
the clock summary results as below:

~# cat /sys/kernel/debug/clk/clk_summary | grep ltdc
        ltdc_px               1        1        0    29700000          0     0  50000         N
              ltdc            0        0        0   133250000          0     0  50000         N

By doing so, pm_runtime_get_sync only increments the clock counter when
the driver was in not active, displaying the right information when the
LTDC is not in use, resulting of the below clock summary after deactivation
of the LTDC.

~# cat /sys/kernel/debug/clk/clk_summary | grep ltdc
        ltdc_px               0        0        0    29700000          0     0  50000         N
              ltdc            0        0        0   133250000          0     0  50000         N

The clocks are activated either by the crtc_set_nofb function or
by the crtc_atomic_enable function. A check of pm_runtime activity must
be done before set clocks on. This check must also be done for others
functions which access registers.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 drivers/gpu/drm/stm/ltdc.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 08b71248044d..bf9d18023698 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -425,10 +425,17 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
 	struct drm_device *ddev = crtc->dev;
+	int ret;
 
 	DRM_DEBUG_DRIVER("\n");
 
-	pm_runtime_get_sync(ddev->dev);
+	if (!pm_runtime_active(ddev->dev)) {
+		ret = pm_runtime_get_sync(ddev->dev);
+		if (ret) {
+			DRM_ERROR("Failed to set mode, cannot get sync\n");
+			return;
+		}
+	}
 
 	/* Sets the background color value */
 	reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
@@ -783,6 +790,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	struct drm_plane_state *newstate = drm_atomic_get_new_plane_state(state,
 									  plane);
 	struct drm_framebuffer *fb = newstate->fb;
+	struct drm_device *ddev = plane->dev;
 	u32 lofs = plane->index * LAY_OFS;
 	u32 x0 = newstate->crtc_x;
 	u32 x1 = newstate->crtc_x + newstate->crtc_w - 1;
@@ -792,6 +800,11 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	u32 val, pitch_in_bytes, line_length, paddr, ahbp, avbp, bpcr;
 	enum ltdc_pix_fmt pf;
 
+	if (!pm_runtime_active(ddev->dev)) {
+		DRM_DEBUG_DRIVER("crtc not activated");
+		return;
+	}
+
 	if (!newstate->crtc || !fb) {
 		DRM_DEBUG_DRIVER("fb or crtc NULL");
 		return;
@@ -897,8 +910,14 @@ static void ltdc_plane_atomic_disable(struct drm_plane *plane,
 	struct drm_plane_state *oldstate = drm_atomic_get_old_plane_state(state,
 									  plane);
 	struct ltdc_device *ldev = plane_to_ltdc(plane);
+	struct drm_device *ddev = plane->dev;
 	u32 lofs = plane->index * LAY_OFS;
 
+	if (!pm_runtime_active(ddev->dev)) {
+		DRM_DEBUG_DRIVER("crtc already deactivated");
+		return;
+	}
+
 	/* disable layer */
 	reg_clear(ldev->regs, LTDC_L1CR + lofs, LXCR_LEN);
 
-- 
2.24.3

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Raphael GALLAIS-POU - foss <raphael.gallais-pou@foss.st.com>
To: Yannick FERTRE - foss <yannick.fertre@foss.st.com>,
	Philippe CORNU - foss <philippe.cornu@foss.st.com>,
	Benjamin Gaignard <benjamin.gaignard@linaro.org>,
	David Airlie <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre TORGUE - foss <alexandre.torgue@foss.st.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: Yannick FERTRE <yannick.fertre@st.com>,
	Marek Vasut <marex@denx.de>,
	Raphael GALLAIS-POU <raphael.gallais-pou@st.com>,
	Philippe CORNU <philippe.cornu@st.com>,
	Raphael GALLAIS-POU - foss <raphael.gallais-pou@foss.st.com>
Subject: [PATCH] drm/stm: ltdc: improve pm_runtime to stop clocks
Date: Tue, 29 Jun 2021 11:58:19 +0000	[thread overview]
Message-ID: <20210629115709.16145-1-raphael.gallais-pou@foss.st.com> (raw)

Bugzilla ticket: https://intbugzilla.st.com/show_bug.cgi?id=60620
Gerrit patch: https://gerrit.st.com/c/mpu/oe/st/linux-stm32/+/208093/

In the LTDC driver, pm_runtime_get_sync was wrongly used and caused the
LTDC pixel clock to be systematically enabled in the clock summary.

After one simple use of the LTDC by activating and deactivating,
the clock summary results as below:

~# cat /sys/kernel/debug/clk/clk_summary | grep ltdc
        ltdc_px               1        1        0    29700000          0     0  50000         N
              ltdc            0        0        0   133250000          0     0  50000         N

By doing so, pm_runtime_get_sync only increments the clock counter when
the driver was in not active, displaying the right information when the
LTDC is not in use, resulting of the below clock summary after deactivation
of the LTDC.

~# cat /sys/kernel/debug/clk/clk_summary | grep ltdc
        ltdc_px               0        0        0    29700000          0     0  50000         N
              ltdc            0        0        0   133250000          0     0  50000         N

The clocks are activated either by the crtc_set_nofb function or
by the crtc_atomic_enable function. A check of pm_runtime activity must
be done before set clocks on. This check must also be done for others
functions which access registers.

Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
---
 drivers/gpu/drm/stm/ltdc.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 08b71248044d..bf9d18023698 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -425,10 +425,17 @@ static void ltdc_crtc_atomic_enable(struct drm_crtc *crtc,
 {
 	struct ltdc_device *ldev = crtc_to_ltdc(crtc);
 	struct drm_device *ddev = crtc->dev;
+	int ret;
 
 	DRM_DEBUG_DRIVER("\n");
 
-	pm_runtime_get_sync(ddev->dev);
+	if (!pm_runtime_active(ddev->dev)) {
+		ret = pm_runtime_get_sync(ddev->dev);
+		if (ret) {
+			DRM_ERROR("Failed to set mode, cannot get sync\n");
+			return;
+		}
+	}
 
 	/* Sets the background color value */
 	reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK);
@@ -783,6 +790,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	struct drm_plane_state *newstate = drm_atomic_get_new_plane_state(state,
 									  plane);
 	struct drm_framebuffer *fb = newstate->fb;
+	struct drm_device *ddev = plane->dev;
 	u32 lofs = plane->index * LAY_OFS;
 	u32 x0 = newstate->crtc_x;
 	u32 x1 = newstate->crtc_x + newstate->crtc_w - 1;
@@ -792,6 +800,11 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	u32 val, pitch_in_bytes, line_length, paddr, ahbp, avbp, bpcr;
 	enum ltdc_pix_fmt pf;
 
+	if (!pm_runtime_active(ddev->dev)) {
+		DRM_DEBUG_DRIVER("crtc not activated");
+		return;
+	}
+
 	if (!newstate->crtc || !fb) {
 		DRM_DEBUG_DRIVER("fb or crtc NULL");
 		return;
@@ -897,8 +910,14 @@ static void ltdc_plane_atomic_disable(struct drm_plane *plane,
 	struct drm_plane_state *oldstate = drm_atomic_get_old_plane_state(state,
 									  plane);
 	struct ltdc_device *ldev = plane_to_ltdc(plane);
+	struct drm_device *ddev = plane->dev;
 	u32 lofs = plane->index * LAY_OFS;
 
+	if (!pm_runtime_active(ddev->dev)) {
+		DRM_DEBUG_DRIVER("crtc already deactivated");
+		return;
+	}
+
 	/* disable layer */
 	reg_clear(ldev->regs, LTDC_L1CR + lofs, LXCR_LEN);
 
-- 
2.24.3

             reply	other threads:[~2021-06-29 11:58 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-29 11:58 Raphael GALLAIS-POU - foss [this message]
2021-06-29 11:58 ` [PATCH] drm/stm: ltdc: improve pm_runtime to stop clocks Raphael GALLAIS-POU - foss
2021-06-29 11:58 ` Raphael GALLAIS-POU - foss
2021-06-30  0:35 ` Marek Vasut
2021-06-30  0:35   ` Marek Vasut
2021-06-30  0:35   ` Marek Vasut
2021-07-02  9:23   ` Raphael Gallais-Pou
2021-07-02  9:23     ` Raphael Gallais-Pou
2021-07-02  9:23     ` Raphael Gallais-Pou
2021-07-02 18:07     ` Marek Vasut
2021-07-02 18:07       ` Marek Vasut
2021-07-02 18:07       ` Marek Vasut
2021-08-17  9:43       ` Raphael Gallais-Pou
2021-08-17  9:43         ` Raphael Gallais-Pou
2021-08-17  9:43         ` Raphael Gallais-Pou
2021-08-19 22:22         ` Marek Vasut
2021-08-19 22:22           ` Marek Vasut
2021-08-19 22:22           ` Marek Vasut
2021-07-06 15:21 ` yannick Fertre
2021-07-06 15:21   ` yannick Fertre
2021-07-06 15:21   ` yannick Fertre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210629115709.16145-1-raphael.gallais-pou@foss.st.com \
    --to=raphael.gallais-pou@foss.st.com \
    --cc=airlied@linux.ie \
    --cc=alexandre.torgue@foss.st.com \
    --cc=benjamin.gaignard@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=marex@denx.de \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=philippe.cornu@foss.st.com \
    --cc=philippe.cornu@st.com \
    --cc=raphael.gallais-pou@st.com \
    --cc=yannick.fertre@foss.st.com \
    --cc=yannick.fertre@st.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.