[PATCH 2/2] drm/arm/malidp: Setting QoS priority for 4k to avoid flicker issue
liviu.dudau at arm.com
liviu.dudau at arm.com
Fri Mar 29 16:10:32 UTC 2019
On Fri, Mar 29, 2019 at 08:20:00AM +0000, Wen He wrote:
>
>
> > -----Original Message-----
> > From: liviu.dudau at arm.com [mailto:liviu.dudau at arm.com]
> > Sent: 2019年3月29日 0:39
> > To: Wen He <wen.he_1 at nxp.com>
> > Cc: brian.starkey at arm.com; malidp at foss.arm.com;
> > dri-devel at lists.freedesktop.org
> > Subject: Re: [PATCH 2/2] drm/arm/malidp: Setting QoS priority for 4k to avoid
> > flicker issue
> >
> > Hi Wen,
> >
> > On Thu, Mar 28, 2019 at 03:56:58AM +0000, Wen He wrote:
> > > When we running DDR benchmark test will able to observed flicker issue
> > > in 4k at 60 resolution on monitor. In LS1028A SoC, need increasing DP500
> > > priority to avoid that issue.
> > >
> > > Signed-off-by: Wen He <wen.he_1 at nxp.com>
> > > ---
> > > drivers/gpu/drm/arm/malidp_hw.c | 13 +++++++++++++
> > > drivers/gpu/drm/arm/malidp_regs.h | 8 ++++++++
> > > 2 files changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c
> > > b/drivers/gpu/drm/arm/malidp_hw.c index 8df12e9a33bb..a5263488eb02
> > > 100644
> > > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > > @@ -378,6 +378,19 @@ static void malidp500_modeset(struct
> > malidp_hw_device *hwdev, struct videomode *
> > > malidp_hw_setbits(hwdev, MALIDP_DISP_FUNC_ILACED,
> > MALIDP_DE_DISPLAY_FUNC);
> > > else
> > > malidp_hw_clearbits(hwdev, MALIDP_DISP_FUNC_ILACED,
> > > MALIDP_DE_DISPLAY_FUNC);
> > > +
> > > +#ifdef CONFIG_ARCH_LAYERSCAPE
> > > + /* Setting QoS for 4k resolution to avoid flicker issue */
> > > + if (mode->hactive == 3840
> > > + && mode->vactive == 2160)
> >
> > I'm not keen on this check, it only enables this for one 3840x2160.
> > What if the output is 2160x3840? Does it matter what pixel clock you use?
> > Can the same issue be seen if (for example) you use 120Hz refresh rate but on
> > a smaller output resolution?
> >
>
> Hi liviu,
Hi Wen,
>
> Let me clearly it.
> Currently, we only supported four resolutions (480p at 60, 720p at 60, 1080 at 60, 4k at 60) for LS1028A Display port.
> All of the refresh rate is 60MHz. so that impossible will be there resolution output's 2160x3840 and refresh rate is 120MHz.
Yes, but you are asking to merge this into the mainline driver which
supports more resolutions than that.
>
> > I think it's worth thinking this in terms of bandwidth and not hardcode for one
> > resolution.
> >
>
> Yes, you are right.
> But only 4k resolution have the flicker issue, so this is a special problem.
> I think that hardcode is better than dynamically adjusting bandwidth.
If you want to do that for your BSP, then it's fine. For mainline, I
think we can do better and think in more generic terms.
>
> > Also, I think setting the QoS bits should be a bit more generic, i.e if the
> > modeset requires a certain bandwidth you should write a value read from a
> > device tree, for example?
> >
> > > + malidp_hw_setbits(hwdev, GREEN_ARQOS_CONFIG
> > > + | RED_ARQOS_CONFIG, MALIDP500_RQOS_QUALITY);
> > > + else
> > > + malidp_hw_clearbits(hwdev, GREEN_ARQOS_CONFIG
> > > + | RED_ARQOS_CONFIG, MALIDP500_RQOS_QUALITY);
> > > +
> > > + malidp_hw_setbits(hwdev, CONFIG_VALID, MALIDP500_CONFIG_VALID);
> >
> > malidp500_modeset() will be called with MALIDP_CFG_VALID set anyway, so
> > you should not need this. I don't know what bit 1 in
> > MALIDP500_CONFIG_VALID does for LS1028A, I don't have that spec.
> >
>
> About this, I got this step from our design team, So just to make sure the configuration works that
> writing value 0x3 to MALIDP500_CONFIG_VALID.
Bit 1 in MALIDP500_CONFIG_VALID is not defined in the Arm spec, so don't
know what the NXP design team has done there, might be worth talking to
them.
>
> Is there can always to valid configuration? If yes, I can remove this line.
Yes, before we enter modeset we set bit 0 of MALIDP500_CONFIG_VALID and we clear
it after modeset is finished. DP500 is a bit weird, you need to look at
CONFIG_VALID as somewhat inverted as a signal from the software point of view.
It is probably easier to think of it in the way the code was written,
i.e. we "enter config mode" before modeset and then "exit config mode"
afterwards.
>
> >
> > > +#endif
> > > }
> > >
> > > int malidp_format_get_bpp(u32 fmt)
> > > diff --git a/drivers/gpu/drm/arm/malidp_regs.h
> > > b/drivers/gpu/drm/arm/malidp_regs.h
> > > index a0dd6e1676a8..8dcf7e9f46ce 100644
> > > --- a/drivers/gpu/drm/arm/malidp_regs.h
> > > +++ b/drivers/gpu/drm/arm/malidp_regs.h
> > > @@ -213,6 +213,14 @@
> > > #define MALIDP500_DC_IRQ_BASE 0x00f00
> > > #define MALIDP500_CONFIG_VALID 0x00f00
> > > #define MALIDP500_CONFIG_ID 0x00fd4
> > > +#ifdef CONFIG_ARCH_LAYERSCAPE
> > > +#define MALIDP500_RQOS_QUALITY 0x00500
> > > +/* Increasing QoS level signal */
> > > +#define GREEN_ARQOS_CONFIG (0xd << 28)
> > > +/* Close to underflow QoS level signal */
> > > +#define RED_ARQOS_CONFIG (0xd << 12)
> > > +#define CONFIG_VALID 0x3
> >
> > What are these values? Is it something NXP has added to the DP500? If so, I
> > think these should have some LAYERSCAPE (or LS) in their name. Also, rather
> > than hardcoding the values, would it not be better to read them form device
> > tree, to allow for fine tuning or variability between IP deployments?
> >
> > Best regards,
> > Liviu
> >
>
> These values are QoS registers of DP500.
> I have already put CONFIG_ARCH_LAYERSCAPE definition to specify architecture.
>
> Regarding your suggestion, I think device tree also can meet this, but that is
> Non-standard writing in here, because not have any properties define to describe QoS
> in arm,malidp device bindings file.
What I was thinking was that you could add the properties and the
documentation ;)
Best regards,
Liviu
>
> Best Regards,
> Wen
> >
> > > +#endif
> > >
> > > /* register offsets and bits specific to DP550/DP650 */
> > > #define MALIDP550_ADDR_SPACE_SIZE 0x10000
> > > --
> > > 2.17.1
> > >
> >
> > --
> > ====================
> > | I would like to |
> > | fix the world, |
> > | but they're not |
> > | giving me the |
> > \ source code! /
> > ---------------
> > ¯\_(ツ)_/¯
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
More information about the dri-devel
mailing list