> -----Original Message-----
> From: liviu.du...@arm.com [mailto:liviu.du...@arm.com]
> Sent: 2019年3月29日 0:39
> To: Wen He <wen.h...@nxp.com>
> Cc: brian.star...@arm.com; mal...@foss.arm.com;
> dri-devel@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@60 resolution on monitor. In LS1028A SoC, need increasing DP500
> > priority to avoid that issue.
> >
> > Signed-off-by: Wen He <wen.h...@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,

Let me clearly it.
Currently, we only supported four resolutions (480p@60, 720p@60, 1080@60, 
4k@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.

> 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.

> 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.

Is there can always to valid configuration? If yes, I can remove this line.

> 
> > +#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.

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!  /
>   ---------------
>     ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to