RE: [PATCH 2/2] drm/arm/malidp: Setting QoS priority for 4k to avoid flicker issue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: liviu.dudau@xxxxxxx <liviu.dudau@xxxxxxx>
> Sent: 2019年3月30日 0:11
> To: Wen He <wen.he_1@xxxxxxx>
> Cc: brian.starkey@xxxxxxx; malidp@xxxxxxxxxxxx;
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxx [mailto:liviu.dudau@xxxxxxx]
> > > Sent: 2019年3月29日 0:39
> > > To: Wen He <wen.he_1@xxxxxxx>
> > > Cc: brian.starkey@xxxxxxx; malidp@xxxxxxxxxxxx;
> > > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > > 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.he_1@xxxxxxx>
> > > > ---
> > > >  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@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.
> 
> 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!  /
>   ---------------
>     ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux