Re: [PATCH 1/2] drm/imagination: Improve firmware power off for layout_mars config

[Brajesh Gupta <Brajesh.Gupta@imgtec.com>]

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