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