Re: [PATCH v5 0/8] drm: rcar-du: Add Color Management Module (CMM)

[Eugeniu Rosca <erosca@de.adit-jv.com>]

Hi,

On Fri, Jun 12, 2020 at 06:10:05PM +0300, Laurent Pinchart wrote:
> On Fri, Jun 12, 2020 at 05:00:32PM +0200, Jacopo Mondi wrote:
> > On Tue, Jun 09, 2020 at 04:29:59PM +0200, Eugeniu Rosca wrote:
> > > On Sun, Jun 07, 2020 at 05:41:58AM +0300, Laurent Pinchart wrote:
> > > > Note that the CMM driver is controlled by the DU driver. As the DU
> > > > driver will reenable the display during resume, it will call
> > > > rcar_du_cmm_setup() at resume time, which will reprogram the CMM. There
> > > > should thus be no need for manual suspend/resume handling in the CMM as
> > > > far as I can tell, but we need to ensure that the CMM is suspended
> > > > before and resumed after the DU. I believe this could be implemented
> > > > using device links.
> > >
> > > Based on below quote [*] from Jacopo's commit [**], isn't the device
> > > link relationship already in place?
> > 
> > Yes, it's in place already.
> > 
> > I added pm_ops to cmm just to be able to printout when suspend/resume
> > happens and the sequence is what comment [*] reports
> > 
> > [  222.909002] rcar_du_pm_suspend:505
> > [  223.145497] rcar_cmm_pm_suspend:193
> > 
> > [  223.208053] rcar_cmm_pm_resume:200
> > [  223.460094] rcar_du_pm_resume:513
> > 
> > However, Laurent mentioned that in his comment here that he expects
> > the opposite sequence to happen (CMM to suspend before and resume after
> > DU).
> > 
> > I still think what is implemented is correct:
> > - CMM is suspended after DU: when CMM is suspended, DU is not feeding
> >   it with data
> > - CMM is resumed before: once DU restart operations CMM is ready to
> >   receive data.
> > 
> > Laurent, what do you think ?
> 
> I think I shouldn't have written the previous e-mail in the middle of
> the night :-) Suspending CMM after DU is obviously correct.

Thanks to Renesas team (kudos to Gotthard and Michael), we've
figured out that below sequence of clock handling (happening during
concurrent suspend and HDMI display unplug) leads to SoC lockup:

cmm1 OFF 	(caused by HDMI unplug)
x21-clock OFF 	(caused by HDMI unplug)
du1 OFF 	(caused by HDMI unplug)
cmm1 ON (caused by suspend to ram, as preparation for CMM register save)
# Freeze happens

That seems to be explained by Chapter 35A.4.3 "Restriction of enabling
clock signal of the CMM" of HW User's manual (Rev.2.00 Jul 2019):

 -----8<-----
 When the clock signal of the CMM is enabled (RMSTPCR7.CMMn or
 SMSTPCR7.CMMn = 0), the clock signal of the DU should be also enabled
 (RMSTPCR7.DUn or SMSTPCR7.DUn = 0).
 -----8<-----

So, the lesson learned from the above is: do not enable the CMMi clock
while the DUi clock is disabled. I expect this to also potentially
give some input w.r.t. what to suspend/resume first, CMM or DU.

> 
> > > [*] Quote from commit [**]
> > >    Enforce the probe and suspend/resume ordering of DU and CMM by
> > >    creating a stateless device link between the two.
> > >
> > > [**] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8de707aeb45241
> > >     ("drm: rcar-du: kms: Initialize CMM instances")

-- 
Best regards,
Eugeniu Rosca