| Message ID | 20190822021135.10288-1-wen.he_1@nxp.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v4,1/2] dt/bindings: display: Add optional property node define for Mali DP500 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Thu, 22 Aug 2019 10:11:34 +0800, Wen He wrote: > Add optional property node 'arm,malidp-arqos-value' for the Mali DP500. > This property describe the ARQoS levels of DP500's QoS signaling. > > Signed-off-by: Wen He <wen.he_1@nxp.com> > --- > Documentation/devicetree/bindings/display/arm,malidp.txt | 3 +++ > 1 file changed, 3 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
> -----Original Message----- > From: Liviu Dudau <liviu.dudau@arm.com> > Sent: 2019年9月6日 22:18 > To: Wen He <wen.he_1@nxp.com> > Cc: linux-devel@linux.nxdi.nxp.com; Brian Starkey <brian.starkey@arm.com>; > David Airlie <airlied@linux.ie>; Daniel Vetter <daniel@ffwll.ch>; Rob Herring > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; > dri-devel@lists.freedesktop.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; Leo Li <leoyang.li@nxp.com> > Subject: [EXT] Re: [v4 2/2] drm/arm/mali-dp: Add display QoS interface > configuration for Mali DP500 > > Caution: EXT Email > > Hi Wen, > > On Thu, Aug 22, 2019 at 10:11:35AM +0800, Wen He wrote: > > Configure the display Quality of service (QoS) levels priority if the > > optional property node "arm,malidp-aqros-value" is defined in DTS file. > > > > QoS signaling using AQROS and AWQOS AXI interface signals, the AQROS > > is driven from the "RQOS" register, so needed to program the RQOS > > register to avoid the high resolutions flicker issue on the LS1028A platform. > > > > Signed-off-by: Wen He <wen.he_1@nxp.com> > > --- > > drivers/gpu/drm/arm/malidp_drv.c | 6 ++++++ > > drivers/gpu/drm/arm/malidp_hw.c | 13 +++++++++++++ > > drivers/gpu/drm/arm/malidp_hw.h | 3 +++ > > drivers/gpu/drm/arm/malidp_regs.h | 10 ++++++++++ > > 4 files changed, 32 insertions(+) > > > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > b/drivers/gpu/drm/arm/malidp_drv.c > > index c27ff456eddc..80e8d15760ac 100644 > > --- a/drivers/gpu/drm/arm/malidp_drv.c > > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > @@ -815,6 +815,12 @@ static int malidp_bind(struct device *dev) > > > > malidp->core_id = version; > > > > + ret = of_property_read_u32(dev->of_node, > > + "arm,malidp-arqos-value", > > + &hwdev->arqos_value); > > + if (ret) > > + hwdev->arqos_value = 0x0; > > + > > /* set the number of lines used for output of RGB data */ > > ret = of_property_read_u8_array(dev->of_node, > > "arm,malidp-output-port-lines", > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c > > b/drivers/gpu/drm/arm/malidp_hw.c index 380be66d4c6e..f90a367a5bc9 > > 100644 > > --- a/drivers/gpu/drm/arm/malidp_hw.c > > +++ b/drivers/gpu/drm/arm/malidp_hw.c > > @@ -374,6 +374,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); > > + > > + /* > > + * Program the RQoS register to avoid high resolutions flicker > > + * issue on the LS1028A. > > + */ > > + if (hwdev->arqos_value) { > > + val = hwdev->arqos_value; > > + > > + if (mode->pixelclock > 148500000) > > + malidp_hw_setbits(hwdev, val, > MALIDP500_RQOS_QUALITY); > > + else > > + malidp_hw_clearbits(hwdev, val, > MALIDP500_RQOS_QUALITY); > > + } > > This application of the arqos_value based on a pixel clock value bothers me, > because it has two problems: > > 1. Some other user of the Mali DP driver can't apply a system QoS value that > they can now specify in the DT, unless the requested pixel clock is bigger than > 145MHz. :( > > 2. (A guess) The flickering issue shows up on a combination of pixelclock and > resolution (i.e. it is a bandwidth problem), but you only address one of the > variables. Haven't tested on the LS1028A yet, but do you know if (theoretically) > it would have a flicker problem doing 640x480@200MHz without the QoS > value? > > How about this instead? > > --8<--------------------------------------------------------------------- > diff --git a/drivers/gpu/drm/arm/malidp_hw.c > b/drivers/gpu/drm/arm/malidp_hw.c index 380be66d4c6eb..e2f96dce13850 > 100644 > --- a/drivers/gpu/drm/arm/malidp_hw.c > +++ b/drivers/gpu/drm/arm/malidp_hw.c > @@ -374,6 +374,22 @@ 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); > + > + /* > + * Program the RQoS register. LS1028A has an issue where screen > will > + * flicker on pixelclocks higher than 148.5MHz but otherwise doesn't > + * want an RQoS value, so special case it for them. > + */ > + if (hwdev->arqos_value) { > + val = hwdev->arqos_value; > + > +#ifdef MALIDP_LS1028A > + if (mode->pixelclock <= 148500000) > + malidp_hw_clearbits(hwdev, val, > MALIDP500_RQOS_QUALITY); > + else > +#endif > + malidp_hw_setbits(hwdev, val, > MALIDP500_RQOS_QUALITY); > + } > } > > int malidp_format_get_bpp(u32 fmt) > --8<--------------------------------------------------------------------- > > And then you need to define a MALIDP_LS1028A in a vendor patch on top of > the kernel source code. That's a good instead, thanks a lot. After discussed with our team, they think should not put the different on the vendor code, benchmark results can accept the small reduce, so here should always apply this change. I will send next version that remove the condition of the pixel clock , thanks a lot. Best Regards, Wen > > Best regards, > Liviu > > > > } > > > > int malidp_format_get_bpp(u32 fmt) > > diff --git a/drivers/gpu/drm/arm/malidp_hw.h > > b/drivers/gpu/drm/arm/malidp_hw.h index 968a65eed371..e4c36bc90bda > > 100644 > > --- a/drivers/gpu/drm/arm/malidp_hw.h > > +++ b/drivers/gpu/drm/arm/malidp_hw.h > > @@ -251,6 +251,9 @@ struct malidp_hw_device { > > > > /* size of memory used for rotating layers, up to two banks available > */ > > u32 rotation_memory[2]; > > + > > + /* priority level of RQOS register used for driven the ARQOS signal */ > > + u32 arqos_value; > > }; > > > > static inline u32 malidp_hw_read(struct malidp_hw_device *hwdev, u32 > > reg) diff --git a/drivers/gpu/drm/arm/malidp_regs.h > > b/drivers/gpu/drm/arm/malidp_regs.h > > index 993031542fa1..514c50dcb74d 100644 > > --- a/drivers/gpu/drm/arm/malidp_regs.h > > +++ b/drivers/gpu/drm/arm/malidp_regs.h > > @@ -210,6 +210,16 @@ > > #define MALIDP500_CONFIG_VALID 0x00f00 > > #define MALIDP500_CONFIG_ID 0x00fd4 > > > > +/* > > + * The quality of service (QoS) register on the DP500. RQOS register > > +values > > + * are driven by the ARQOS signal, using AXI transacations, dependent > > +on the > > + * FIFO input level. > > + * The RQOS register can also set QoS levels for: > > + * - RED_ARQOS @ A 4-bit signal value for close to underflow > conditions > > + * - GREEN_ARQOS @ A 4-bit signal value for normal conditions > > + */ > > +#define MALIDP500_RQOS_QUALITY 0x00500 > > + > > /* register offsets and bits specific to DP550/DP650 */ > > #define MALIDP550_ADDR_SPACE_SIZE 0x10000 > > #define MALIDP550_DE_CONTROL 0x00010 > > -- > > 2.17.1 > > > > -- > ==================== > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --------------- > ¯\_(ツ)_/¯
diff --git a/Documentation/devicetree/bindings/display/arm,malidp.txt b/Documentation/devicetree/bindings/display/arm,malidp.txt index 2f7870983ef1..7a97a2b48c2a 100644 --- a/Documentation/devicetree/bindings/display/arm,malidp.txt +++ b/Documentation/devicetree/bindings/display/arm,malidp.txt @@ -37,6 +37,8 @@ Optional properties: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt) to be used for the framebuffer; if not present, the framebuffer may be located anywhere in memory. + - arm,malidp-arqos-high-level: integer of u32 value describing the ARQoS + levels of DP500's QoS signaling. Example: @@ -54,6 +56,7 @@ Example: clocks = <&oscclk2>, <&fpgaosc0>, <&fpgaosc1>, <&fpgaosc1>; clock-names = "pxlclk", "mclk", "aclk", "pclk"; arm,malidp-output-port-lines = /bits/ 8 <8 8 8>; + arm,malidp-arqos-high-level = <0xd000d000>; port { dp0_output: endpoint { remote-endpoint = <&tda998x_2_input>;
Add optional property node 'arm,malidp-arqos-value' for the Mali DP500. This property describe the ARQoS levels of DP500's QoS signaling. Signed-off-by: Wen He <wen.he_1@nxp.com> --- Documentation/devicetree/bindings/display/arm,malidp.txt | 3 +++ 1 file changed, 3 insertions(+)