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

[Laurent Pinchart]

Hello,

On Fri, Jun 05, 2020 at 03:53:15PM +0200, Jacopo Mondi wrote:
> On Fri, Jun 05, 2020 at 03:41:24PM +0200, Eugeniu Rosca wrote:
> > On Fri, Jun 05, 2020 at 03:29:00PM +0200, Jacopo Mondi wrote:
> >> On Wed, May 27, 2020 at 09:15:55AM +0200, Eugeniu Rosca wrote:
> >>> Could you kindly share the cross compilation steps for your kmsxx fork?
> >>
> >> I usually build it on the target :)
> >
> > Interesting approach. With ARM getting more and more potent, why not? :)
> 
> For 'small' utilities like kmsxx it's doable
> 
> >>> Just out of curiosity, have you ever tried to pull the display's HDMI
> >>> cable while reading from CM2_LUT_TBL?
> >>
> >> Ahem, not really :) Did I get you right, you mean disconnecting the
> >> HDMI cable from the board ?
> >
> > Right.
> 
> So, no, I have not tried. Do you see any intersting failure with the
> mainline version ?

Jacopo, would you be able to give this a try ?

> >>> At least with the out-of-tree CMM implementation [*], this sends the
> >>> R-Car3 reference targets into an unrecoverable freeze, with no lockup
> >>> reported by the kernel (i.e. looks like an serious HW issue).
> >>>
> >>>> CMM functionalities are retained between suspend/resume cycles (tested with
> >>>> suspend-to-idle) without requiring a re-programming of the LUT tables.
> >>>
> >>> Hmm. Is this backed up by any statement in the HW User's manual?
> >>> This comes in contrast with the original Renesas CMM implementation [**]
> >>> which does make use of suspend (where the freeze actually happens).
> >>>
> >>> Can we infer, based on your statement, that we could also get rid of
> >>> the suspend callback in [**]?
> >>
> >> As Geert (thanks) explained what I've tested with is suspend-to-idle,
> >> which retains the state of the LUT tables (and I assume other
> >> not-yet-implemented CMM features, like CLU). I recall the out-of-tree
> >> driver has suspend/resume routines but I never really tested that.
> >
> > I see. JFYI, there is a flaw in the suspend handling in the out-of-tree
> > CMM patch [*], which renders the SoC unresponsive on HDMI hotplug. The
> > fix is currently under review. Hopefully it will make its way to [*]
> > in the nearest future. Just to keep in mind for the moment when CMM
> > s2ram will become a mainline feature.
> 
> Thanks, let's keep this in mind. Next week I'll run a few tests again
> with s2ram and will get back to you.

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.

> >>> [*] https://github.com/renesas-rcar/du_cmm
> >>> [**] https://github.com/renesas-rcar/du_cmm/blob/c393ed49834bdbc/meta-rcar-gen3/recipes-kernel/linux/linux-renesas/0001-drm-rcar-du-Add-DU-CMM-support.patch#L1912

-- 
Regards,

Laurent Pinchart