diff mbox series

video: rk_vop: Fix wrong bpix for frame buffer

Message ID 20200624151843.120134-1-jagan@amarulasolutions.com
State New
Headers
Series video: rk_vop: Fix wrong bpix for frame buffer |

Commit Message

Jagan Teki June 24, 2020, 3:18 p.m. UTC 
Video framework would use plat size as a frame buffer
pointer in rockchip video drivers.

Typical frame buffer pointer would compute based on
maximum resolutions supporting followed by bits per
pixel value. Right now the value 4 (VIDEO_BPP16) is
using on this computation even though the HDMI vop_id
assigned as VIDEO_BPP32.

This results below synchronous abort while clearing the
frame buffer to the background color.

"Synchronous Abort" handler, esr 0x96000045
elr: 0000000000236ff0 lr : 0000000000236f74 (reloc)
elr: 00000000f6f6eff0 lr : 00000000f6f6ef74
x0 : 00000000f8000000 x1 : 0000000000000000
x2 : 00000000f97a4000 x3 : 00000000ff1a0000
x4 : 00000000ff1a0000 x5 : 0000000000000035
x6 : 000000000000000a x7 : 00000000f4f1fe50
x8 : 0000000000000000 x9 : 0000000000000008
x10: 00000000ffffffd8 x11: 0000000000000006
x12: 000000000001869f x13: 0000000000005dc0
x14: 0000000000000000 x15: 00000000ffffffff
x16: 0000000000000001 x17: 0000000000000032
x18: 00000000f4f31dc0 x19: 00000000f4f47160
x20: 00000000f6fb5814 x21: 00000000f4f1feb0
x22: 00000000f6fea748 x23: 00000000f6fea748
x24: 00000000f4f47108 x25: 00000000f4f47110
x26: 00000000f4f46ed0 x27: 00000000f4f47118
x28: 0000000000000002 x29: 00000000f4f1fe50

Reproduced on,
Board: roc-rk3399-pc
Video Out: HDMI with 3480x2160 resolution.

Fix this by using maximum bpix value which is VIDEO_BPP32
to satisfy all  vop_id connections.

Reported-by: Da Xue <da@lessconfused.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/video/rockchip/rk_vop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jagan Teki June 28, 2020, 3:06 p.m. UTC | #1 
On Wed, Jun 24, 2020 at 8:48 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Video framework would use plat size as a frame buffer
> pointer in rockchip video drivers.
>
> Typical frame buffer pointer would compute based on
> maximum resolutions supporting followed by bits per
> pixel value. Right now the value 4 (VIDEO_BPP16) is
> using on this computation even though the HDMI vop_id
> assigned as VIDEO_BPP32.
>
> This results below synchronous abort while clearing the
> frame buffer to the background color.
>
> "Synchronous Abort" handler, esr 0x96000045
> elr: 0000000000236ff0 lr : 0000000000236f74 (reloc)
> elr: 00000000f6f6eff0 lr : 00000000f6f6ef74
> x0 : 00000000f8000000 x1 : 0000000000000000
> x2 : 00000000f97a4000 x3 : 00000000ff1a0000
> x4 : 00000000ff1a0000 x5 : 0000000000000035
> x6 : 000000000000000a x7 : 00000000f4f1fe50
> x8 : 0000000000000000 x9 : 0000000000000008
> x10: 00000000ffffffd8 x11: 0000000000000006
> x12: 000000000001869f x13: 0000000000005dc0
> x14: 0000000000000000 x15: 00000000ffffffff
> x16: 0000000000000001 x17: 0000000000000032
> x18: 00000000f4f31dc0 x19: 00000000f4f47160
> x20: 00000000f6fb5814 x21: 00000000f4f1feb0
> x22: 00000000f6fea748 x23: 00000000f6fea748
> x24: 00000000f4f47108 x25: 00000000f4f47110
> x26: 00000000f4f46ed0 x27: 00000000f4f47118
> x28: 0000000000000002 x29: 00000000f4f1fe50
>
> Reproduced on,
> Board: roc-rk3399-pc
> Video Out: HDMI with 3480x2160 resolution.
>
> Fix this by using maximum bpix value which is VIDEO_BPP32
> to satisfy all  vop_id connections.
>
> Reported-by: Da Xue <da@lessconfused.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/video/rockchip/rk_vop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c
> index ff1a80384d..3bae22a5bc 100644
> --- a/drivers/video/rockchip/rk_vop.c
> +++ b/drivers/video/rockchip/rk_vop.c
> @@ -425,7 +425,7 @@ int rk_vop_bind(struct udevice *dev)
>  {
>         struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
>
> -       plat->size = 4 * (CONFIG_VIDEO_ROCKCHIP_MAX_XRES *
> +       plat->size = VIDEO_BPP32 * (CONFIG_VIDEO_ROCKCHIP_MAX_XRES *
>                           CONFIG_VIDEO_ROCKCHIP_MAX_YRES);
>
>         return 0;
> --
> 2.25.1
>

Any comments on this fix? it needs to be in the release in order to
work 4K HDMI. I did tests lower than 4K resolution as well.


Jagan.
Anatolij Gustschin June 28, 2020, 7:12 p.m. UTC | #2 
Hi Jagan,

On Wed, 24 Jun 2020 20:48:43 +0530
Jagan Teki jagan@amarulasolutions.com wrote:
...
> @@ -425,7 +425,7 @@ int rk_vop_bind(struct udevice *dev)
>  {
>  	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
>  
> -	plat->size = 4 * (CONFIG_VIDEO_ROCKCHIP_MAX_XRES *
> +	plat->size = VIDEO_BPP32 * (CONFIG_VIDEO_ROCKCHIP_MAX_XRES *
>  			  CONFIG_VIDEO_ROCKCHIP_MAX_YRES);

Factor 4 here is actually correct (4 bytes/pixel * xres * yres),
so the problem must be elsewhere. Are you sure that the detected
display resolution matches the values in CONFIG_VIDEO_ROCKCHIP_MAX_*
options in your .config?

Using VIDEO_BPP32 enum here is wrong, VIDEO_BPP32 is log2 value of 32BPP.
We could use VNBYTES(VIDEO_BPP32) for readability, but it won't fix the
problem you see.

--
Anatolij
Anatolij Gustschin June 28, 2020, 7:19 p.m. UTC | #3 
On Sun, 28 Jun 2020 20:36:46 +0530
Jagan Teki jagan@amarulasolutions.com wrote:
...
> Any comments on this fix? it needs to be in the release in order to
> work 4K HDMI. I did tests lower than 4K resolution as well.

I'm afraid that this is not the right fix, so I skipped this
patch in the video pull request for now.

--
Anatolij
diff mbox series

Patch

diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c
index ff1a80384d..3bae22a5bc 100644
--- a/drivers/video/rockchip/rk_vop.c
+++ b/drivers/video/rockchip/rk_vop.c
@@ -425,7 +425,7 @@  int rk_vop_bind(struct udevice *dev)
 {
 	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
 
-	plat->size = 4 * (CONFIG_VIDEO_ROCKCHIP_MAX_XRES *
+	plat->size = VIDEO_BPP32 * (CONFIG_VIDEO_ROCKCHIP_MAX_XRES *
 			  CONFIG_VIDEO_ROCKCHIP_MAX_YRES);
 
 	return 0;