mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/imagination: Improve power off sequence
@ 2026-03-05 11:06 Brajesh Gupta
  2026-03-05 11:06 ` [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config Brajesh Gupta
  2026-03-05 11:06 ` [PATCH 2/2] drm/imagination: Skip 2nd thread DM association for non META Firmware Brajesh Gupta
  0 siblings, 2 replies; 7+ messages in thread
From: Brajesh Gupta @ 2026-03-05 11:06 UTC (permalink / raw)
  To: Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Alessio Belle,
	Alexandru Dadu
  Cc: dri-devel, linux-kernel, Brajesh Gupta

Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
Brajesh Gupta (2):
      drm/imagination: Improve firmware power off for layout_mars config
      drm/imagination: Skip 2nd thread DM association for non META Firmware

 drivers/gpu/drm/imagination/pvr_fw_startstop.c | 100 ++++++++++++++++---------
 1 file changed, 66 insertions(+), 34 deletions(-)
---
base-commit: 68b271a3a94cfd6c7695a96b6398b52feb89e2c2
change-id: 20260304-b4-staging-layout_mars_base-dfbbf1a53b3c

Best regards,
-- 
Brajesh Gupta <brajesh.gupta@imgtec.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config
  2026-03-05 11:06 [PATCH 0/2] drm/imagination: Improve power off sequence Brajesh Gupta
@ 2026-03-05 11:06 ` Brajesh Gupta
  2026-03-11 11:37   ` Matt Coster
  2026-03-05 11:06 ` [PATCH 2/2] drm/imagination: Skip 2nd thread DM association for non META Firmware Brajesh Gupta
  1 sibling, 1 reply; 7+ messages in thread
From: Brajesh Gupta @ 2026-03-05 11:06 UTC (permalink / raw)
  To: Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Alessio Belle,
	Alexandru Dadu
  Cc: dri-devel, linux-kernel, Brajesh Gupta

In layout_mars HW config, Firmware MCU moved from SideKick to new Mars
domain so Firmware takes care of powering down Sidekick/Jones and SLC.
Skip checks for those from kernel and check idle bits for Firmware MCU
and system arbiter excluding SOCIF.

Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_fw_startstop.c | 86 +++++++++++++++++---------
 1 file changed, 58 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_fw_startstop.c b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
index dcbb9903e791..ce089f51f06a 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_startstop.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
@@ -208,19 +208,31 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
 				       ~(ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN |
 					 ROGUE_CR_SIDEKICK_IDLE_SOCIF_EN |
 					 ROGUE_CR_SIDEKICK_IDLE_HOSTIF_EN);
-	bool skip_garten_idle = false;
+	u64 layout_mars_value = 0;
+	bool layout_mars = false;
 	u32 reg_value;
 	int err;
 
+	if (PVR_FEATURE_VALUE(pvr_dev, layout_mars, &layout_mars_value) == 0)
+		layout_mars = layout_mars_value > 0;
+
 	/*
-	 * Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper.
-	 * For cores with the LAYOUT_MARS feature, SIDEKICK would have been
+	 * For cores with the LAYOUT_MARS feature, SIDEKICK and SLC would have been
 	 * powered down by the FW.
 	 */
-	err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
-				sidekick_idle_mask, POLL_TIMEOUT_USEC);
-	if (err)
-		return err;
+	if (!layout_mars) {
+		/* Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper. */
+		err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
+					sidekick_idle_mask, POLL_TIMEOUT_USEC);
+		if (err)
+			return err;
+
+		/* Wait for SLC to signal IDLE. */
+		err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SLC_IDLE, ROGUE_CR_SLC_IDLE_MASKFULL,
+					 ROGUE_CR_SLC_IDLE_MASKFULL, POLL_TIMEOUT_USEC);
+		if (err)
+			return err;
+	}
 
 	/* Unset MTS DM association with threads. */
 	pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD0_DM_ASSOC,
@@ -229,6 +241,7 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
 	pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC,
 		       ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_MASKFULL &
 		       ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_DM_ASSOC_CLRMSK);
+
 	pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC,
 		       ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_MASKFULL &
 		       ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
@@ -270,25 +283,23 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
 		return err;
 
 	/*
-	 * Wait for SLC to signal IDLE.
-	 * For cores with the LAYOUT_MARS feature, SLC would have been powered
-	 * down by the FW.
+	 * For cores with the LAYOUT_MARS feature, SIDEKICK and SLC would have been
+	 * powered down by the FW.
 	 */
-	err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SLC_IDLE,
-				ROGUE_CR_SLC_IDLE_MASKFULL,
-				ROGUE_CR_SLC_IDLE_MASKFULL, POLL_TIMEOUT_USEC);
-	if (err)
-		return err;
+	if (!layout_mars) {
+		/* Wait for SLC to signal IDLE. */
+		err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SLC_IDLE,
+					ROGUE_CR_SLC_IDLE_MASKFULL,
+					ROGUE_CR_SLC_IDLE_MASKFULL, POLL_TIMEOUT_USEC);
+		if (err)
+			return err;
 
-	/*
-	 * Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper.
-	 * For cores with the LAYOUT_MARS feature, SIDEKICK would have been powered
-	 * down by the FW.
-	 */
-	err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
-				sidekick_idle_mask, POLL_TIMEOUT_USEC);
-	if (err)
-		return err;
+		/* Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper. */
+		err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
+					sidekick_idle_mask, POLL_TIMEOUT_USEC);
+		if (err)
+			return err;
+	}
 
 	if (pvr_dev->fw_dev.processor_type == PVR_FW_PROCESSOR_TYPE_META) {
 		err = pvr_meta_cr_read32(pvr_dev, META_CR_TxVECINT_BHALT, &reg_value);
@@ -300,11 +311,30 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
 		 * Wrapper if there is no debugger attached (TxVECINT_BHALT =
 		 * 0x0).
 		 */
-		if (reg_value)
-			skip_garten_idle = true;
-	}
+		if (!reg_value) {
+			err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE,
+						ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
+						ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
+						POLL_TIMEOUT_USEC);
+			if (err)
+				return err;
+		}
+	} else if (layout_mars) {
+		/*
+		 * As FW core has been moved from SIDEKICK to the new MARS domain, checking
+		 * idle bits for CPU & System Arbiter excluding SOCIF which will never be
+		 * idle if Host polling on this register
+		 */
+		err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_MARS_IDLE,
+					ROGUE_CR_MARS_IDLE_CPU_EN |
+					ROGUE_CR_MARS_IDLE_MH_SYSARB0_EN,
+					ROGUE_CR_MARS_IDLE_CPU_EN |
+					ROGUE_CR_MARS_IDLE_MH_SYSARB0_EN,
+					POLL_TIMEOUT_USEC);
 
-	if (!skip_garten_idle) {
+		if (err)
+			return err;
+	} else {
 		err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE,
 					ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
 					ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,

-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drm/imagination: Skip 2nd thread DM association for non META Firmware
  2026-03-05 11:06 [PATCH 0/2] drm/imagination: Improve power off sequence Brajesh Gupta
  2026-03-05 11:06 ` [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config Brajesh Gupta
@ 2026-03-05 11:06 ` Brajesh Gupta
  2026-03-11 11:41   ` Matt Coster
  1 sibling, 1 reply; 7+ messages in thread
From: Brajesh Gupta @ 2026-03-05 11:06 UTC (permalink / raw)
  To: Frank Binns, Matt Coster, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Alessio Belle,
	Alexandru Dadu
  Cc: dri-devel, linux-kernel, Brajesh Gupta

Only a META firmware can have two threads.

Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_fw_startstop.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_fw_startstop.c b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
index ce089f51f06a..3bca57cbaaf0 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_startstop.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
@@ -242,12 +242,14 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
 		       ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_MASKFULL &
 		       ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_DM_ASSOC_CLRMSK);
 
-	pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC,
-		       ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_MASKFULL &
-		       ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
-	pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC,
-		       ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_MASKFULL &
-		       ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
+	if (pvr_dev->fw_dev.processor_type == PVR_FW_PROCESSOR_TYPE_META) {
+		pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC,
+			       ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_MASKFULL &
+			       ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
+		pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC,
+			       ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_MASKFULL &
+			       ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
+	}
 
 	/* Extra Idle checks. */
 	err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_BIF_STATUS_MMU, 0,

-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config
  2026-03-05 11:06 ` [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config Brajesh Gupta
@ 2026-03-11 11:37   ` Matt Coster
  2026-03-13  6:37     ` Brajesh Gupta
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Coster @ 2026-03-11 11:37 UTC (permalink / raw)
  To: Brajesh Gupta
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Frank Binns, Alessio Belle,
	Alexandru Dadu, dri-devel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 7761 bytes --]

Hi Brajesh,

Just a couple minor comments but this otherwise looks good to me.

On 05/03/2026 11:06, Brajesh Gupta wrote:
> In layout_mars HW config, Firmware MCU moved from SideKick to new Mars
> domain so Firmware takes care of powering down Sidekick/Jones and SLC.

I don't know which is correct, but I know we should probably be
consistent with the capitlisation of SideKick/Sidekick.

> Skip checks for those from kernel and check idle bits for Firmware MCU
> and system arbiter excluding SOCIF.
> 
> Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
> ---
>  drivers/gpu/drm/imagination/pvr_fw_startstop.c | 86 +++++++++++++++++---------
>  1 file changed, 58 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_startstop.c b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> index dcbb9903e791..ce089f51f06a 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> @@ -208,19 +208,31 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
>                                        ~(ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN |
>                                          ROGUE_CR_SIDEKICK_IDLE_SOCIF_EN |
>                                          ROGUE_CR_SIDEKICK_IDLE_HOSTIF_EN);
> -       bool skip_garten_idle = false;
> +       u64 layout_mars_value = 0;
> +       bool layout_mars = false;
>         u32 reg_value;
>         int err;
> 
> +       if (PVR_FEATURE_VALUE(pvr_dev, layout_mars, &layout_mars_value) == 0)
> +               layout_mars = layout_mars_value > 0;
> +
>         /*
> -        * Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper.
> -        * For cores with the LAYOUT_MARS feature, SIDEKICK would have been
> +        * For cores with the LAYOUT_MARS feature, SIDEKICK and SLC would have been
>          * powered down by the FW.
>          */
> -       err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
> -                               sidekick_idle_mask, POLL_TIMEOUT_USEC);
> -       if (err)
> -               return err;
> +       if (!layout_mars) {
> +               /* Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper. */
> +               err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
> +                                       sidekick_idle_mask, POLL_TIMEOUT_USEC);
> +               if (err)
> +                       return err;
> +
> +               /* Wait for SLC to signal IDLE. */
> +               err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SLC_IDLE, ROGUE_CR_SLC_IDLE_MASKFULL,
> +                                        ROGUE_CR_SLC_IDLE_MASKFULL, POLL_TIMEOUT_USEC);
> +               if (err)
> +                       return err;
> +       }
> 
>         /* Unset MTS DM association with threads. */
>         pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD0_DM_ASSOC,
> @@ -229,6 +241,7 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
>         pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC,
>                        ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_MASKFULL &
>                        ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_DM_ASSOC_CLRMSK);
> +

I think this might be a remnant from the second patch, can you drop it?

>         pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC,
>                        ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_MASKFULL &
>                        ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
> @@ -270,25 +283,23 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
>                 return err;
> 
>         /*
> -        * Wait for SLC to signal IDLE.
> -        * For cores with the LAYOUT_MARS feature, SLC would have been powered
> -        * down by the FW.
> +        * For cores with the LAYOUT_MARS feature, SIDEKICK and SLC would have been
> +        * powered down by the FW.
>          */
> -       err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SLC_IDLE,
> -                               ROGUE_CR_SLC_IDLE_MASKFULL,
> -                               ROGUE_CR_SLC_IDLE_MASKFULL, POLL_TIMEOUT_USEC);
> -       if (err)
> -               return err;
> +       if (!layout_mars) {
> +               /* Wait for SLC to signal IDLE. */
> +               err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SLC_IDLE,
> +                                       ROGUE_CR_SLC_IDLE_MASKFULL,
> +                                       ROGUE_CR_SLC_IDLE_MASKFULL, POLL_TIMEOUT_USEC);
> +               if (err)
> +                       return err;
> 
> -       /*
> -        * Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper.
> -        * For cores with the LAYOUT_MARS feature, SIDEKICK would have been powered
> -        * down by the FW.
> -        */
> -       err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
> -                               sidekick_idle_mask, POLL_TIMEOUT_USEC);
> -       if (err)
> -               return err;
> +               /* Wait for Sidekick/Jones to signal IDLE except for the Garten Wrapper. */
> +               err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE, sidekick_idle_mask,
> +                                       sidekick_idle_mask, POLL_TIMEOUT_USEC);
> +               if (err)
> +                       return err;
> +       }
> 
>         if (pvr_dev->fw_dev.processor_type == PVR_FW_PROCESSOR_TYPE_META) {
>                 err = pvr_meta_cr_read32(pvr_dev, META_CR_TxVECINT_BHALT, &reg_value);
> @@ -300,11 +311,30 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
>                  * Wrapper if there is no debugger attached (TxVECINT_BHALT =
>                  * 0x0).
>                  */
> -               if (reg_value)
> -                       skip_garten_idle = true;
> -       }
> +               if (!reg_value) {
> +                       err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE,
> +                                               ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
> +                                               ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
> +                                               POLL_TIMEOUT_USEC);
> +                       if (err)
> +                               return err;

This duplicates the polling of CR_SIDEKICK_IDLE. Can you move it down
outside this laddered if/elseif block, and make it conditional on a new
variable poll_sidekick_idle_at_end or similar?

Cheers,
Matt

> +               }
> +       } else if (layout_mars) {
> +               /*
> +                * As FW core has been moved from SIDEKICK to the new MARS domain, checking
> +                * idle bits for CPU & System Arbiter excluding SOCIF which will never be
> +                * idle if Host polling on this register
> +                */
> +               err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_MARS_IDLE,
> +                                       ROGUE_CR_MARS_IDLE_CPU_EN |
> +                                       ROGUE_CR_MARS_IDLE_MH_SYSARB0_EN,
> +                                       ROGUE_CR_MARS_IDLE_CPU_EN |
> +                                       ROGUE_CR_MARS_IDLE_MH_SYSARB0_EN,
> +                                       POLL_TIMEOUT_USEC);
> 
> -       if (!skip_garten_idle) {
> +               if (err)
> +                       return err;
> +       } else {
>                 err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE,
>                                         ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
>                                         ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
> 
> --
> 2.43.0
> 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/imagination: Skip 2nd thread DM association for non META Firmware
  2026-03-05 11:06 ` [PATCH 2/2] drm/imagination: Skip 2nd thread DM association for non META Firmware Brajesh Gupta
@ 2026-03-11 11:41   ` Matt Coster
  2026-03-13  6:27     ` Brajesh Gupta
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Coster @ 2026-03-11 11:41 UTC (permalink / raw)
  To: Brajesh Gupta
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Frank Binns, Alessio Belle,
	Alexandru Dadu, dri-devel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2651 bytes --]

On 05/03/2026 11:06, Brajesh Gupta wrote:
> Only a META firmware can have two threads.

If this is invalid behaviour on non-META firwmare processors, should
this be considered a fix (and gain a Fixes: tag)? I guess technically it
doesn't fix the commit that added these lines[1] since that was only
adding support for META. Would it be fixing the follow up to add MIPS
support[2]? I'm not sure. Maybe it doesn't even need a Fixes: tag.

> 
> Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>

Either way, the content is:

Reviewed-by: Matt Coster <matt.coster@imgtec.com>

[1]: commit cc1aeedb98ad3 ("drm/imagination: Implement firmware infrastructure and META FW support")
[2]: commit 927f3e0253c11 ("drm/imagination: Implement MIPS firmware processor and MMU support")

> ---
>  drivers/gpu/drm/imagination/pvr_fw_startstop.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_fw_startstop.c b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> index ce089f51f06a..3bca57cbaaf0 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> @@ -242,12 +242,14 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
>                        ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_MASKFULL &
>                        ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_DM_ASSOC_CLRMSK);
> 
> -       pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC,
> -                      ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_MASKFULL &
> -                      ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
> -       pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC,
> -                      ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_MASKFULL &
> -                      ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
> +       if (pvr_dev->fw_dev.processor_type == PVR_FW_PROCESSOR_TYPE_META) {
> +               pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC,
> +                              ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_MASKFULL &
> +                              ROGUE_CR_MTS_INTCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
> +               pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC,
> +                              ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_MASKFULL &
> +                              ROGUE_CR_MTS_BGCTX_THREAD1_DM_ASSOC_DM_ASSOC_CLRMSK);
> +       }
> 
>         /* Extra Idle checks. */
>         err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_BIF_STATUS_MMU, 0,
> 
> --
> 2.43.0
> 


-- 
Matt Coster
E: matt.coster@imgtec.com

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/imagination: Skip 2nd thread DM association for non META Firmware
  2026-03-11 11:41   ` Matt Coster
@ 2026-03-13  6:27     ` Brajesh Gupta
  0 siblings, 0 replies; 7+ messages in thread
From: Brajesh Gupta @ 2026-03-13  6:27 UTC (permalink / raw)
  To: Matt Coster
  Cc: tzimmermann, simona, dri-devel, airlied, Frank Binns,
	Alessio Belle, maarten.lankhorst, Alexandru Dadu, mripard,
	linux-kernel

On Wed, 2026-03-11 at 11:41 +0000, Matt Coster wrote:
> On 05/03/2026 11:06, Brajesh Gupta wrote:
> > Only a META firmware can have two threads.
> 
> If this is invalid behaviour on non-META firwmare processors, should
> this be considered a fix (and gain a Fixes: tag)? I guess technically it
> doesn't fix the commit that added these lines[1] since that was only
> adding support for META. Would it be fixing the follow up to add MIPS
> support[2]? I'm not sure. Maybe it doesn't even need a Fixes: tag.
> 
Those operations are no-op for non-META firmware. So an improvement and not
fixing.
Change is applicable for any firmware which has one thread including MIPS. And
no additional fixes for MIPS.

A version 2 will be generated as per patch 1 update.

--
Brajesh Gupta
E: brajesh.gupta@imgtec.com
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config
  2026-03-11 11:37   ` Matt Coster
@ 2026-03-13  6:37     ` Brajesh Gupta
  0 siblings, 0 replies; 7+ messages in thread
From: Brajesh Gupta @ 2026-03-13  6:37 UTC (permalink / raw)
  To: Matt Coster
  Cc: tzimmermann, simona, dri-devel, airlied, Frank Binns,
	Alessio Belle, maarten.lankhorst, Alexandru Dadu, mripard,
	linux-kernel

On Wed, 2026-03-11 at 11:37 +0000, Matt Coster wrote:
> Hi Brajesh,
> 
> Just a couple minor comments but this otherwise looks good to me.
> 
> On 05/03/2026 11:06, Brajesh Gupta wrote:
> > In layout_mars HW config, Firmware MCU moved from SideKick to new Mars
> > domain so Firmware takes care of powering down Sidekick/Jones and SLC.
> 
> I don't know which is correct, but I know we should probably be
> consistent with the capitlisation of SideKick/Sidekick.

Updated to Sidekick.

> >         /* Unset MTS DM association with threads. */
> >         pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_INTCTX_THREAD0_DM_ASSOC,
> > @@ -229,6 +241,7 @@ pvr_fw_stop(struct pvr_device *pvr_dev)
> >         pvr_cr_write32(pvr_dev, ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC,
> >                        ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_MASKFULL &
> >                        ROGUE_CR_MTS_BGCTX_THREAD0_DM_ASSOC_DM_ASSOC_CLRMSK);
> > +
> 
> I think this might be a remnant from the second patch, can you drop it?

Will be dropped.

> >                  * 0x0).
> >                  */
> > -               if (reg_value)
> > -                       skip_garten_idle = true;
> > -       }
> > +               if (!reg_value) {
> > +                       err = pvr_cr_poll_reg32(pvr_dev, ROGUE_CR_SIDEKICK_IDLE,
> > +                                               ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
> > +                                               ROGUE_CR_SIDEKICK_IDLE_GARTEN_EN,
> > +                                               POLL_TIMEOUT_USEC);
> > +                       if (err)
> > +                               return err;
> 
> This duplicates the polling of CR_SIDEKICK_IDLE. Can you move it down
> outside this laddered if/elseif block, and make it conditional on a new
> variable poll_sidekick_idle_at_end or similar?
> 

Will update as per suggestion.

--
Brajesh Gupta
E: brajesh.gupta@imgtec.com 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-03-13  6:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-05 11:06 [PATCH 0/2] drm/imagination: Improve power off sequence Brajesh Gupta
2026-03-05 11:06 ` [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config Brajesh Gupta
2026-03-11 11:37   ` Matt Coster
2026-03-13  6:37     ` Brajesh Gupta
2026-03-05 11:06 ` [PATCH 2/2] drm/imagination: Skip 2nd thread DM association for non META Firmware Brajesh Gupta
2026-03-11 11:41   ` Matt Coster
2026-03-13  6:27     ` Brajesh Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®