[PATCH 2/2] drm/arm/malidp: Setting QoS priority for 4k to avoid flicker issue
Wen He
wen.he_1 at nxp.com
Wed Jul 17 06:26:40 UTC 2019
> -----Original Message-----
> From: liviu.dudau at arm.com <liviu.dudau at arm.com>
> Sent: 2019年3月30日 0:11
> 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
>
> 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.
Hi Liviu,
This patch is not a rigorous implementation for the mainline, I will use the new
Implementation to send next version of the patch.
Thanks for these comments.
Best Regards,
Wen
>
> >
> > >
> > > > +#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