[PATCH v3] media: rockchip: rga: fix rotation on rev0 hardware

[PATCH v3] media: rockchip: rga: fix rotation on rev0 hardware

[John Keeping]

On RK3288 where the RGA reports version zero [1], rotating a buffer
gives corrupt output where one edge will be truncated.  This is a result
of some adjustment done specifically for hardware revision zero.  But it
seems this is not necessary and there is no evidence for why this
adjustment should be applied on these hardware revisions.

Comparing the TRMs between RK3288 with revision zero and RK3588 with
revision 3.2 there is evidence of new features supported by later
hardware, but nothing indicating any difference in behaviour for
rotation which is supported by all hardware revisions.

The logic here looks wrong anyway, since it's bizarre that minor
revision zero would need the adjustment for _all_ major revisions.  This
was added with the initial version of the driver in commit f7e7b48e6d796
("[media] rockchip/rga: v4l2 m2m support") so there is no detail in the
Git history to indicate why this special-case might be needed.

[1] Comparing with the vendor code, it seems this is actually 1.3 with
    the hardware reporting zero more by coincidence than design since the
    version register is not defined for RK3288.

Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
---
base-commit: 2579cbe68005f46fc7f8f95364f6b101b07b9d1c

v3:
- Rebase on media-committers/fixes and add base-commit to hopefully make
  CI happy (patch content is completely unchanged)
v2:
- Remove variable that is unused after this change
---
 drivers/media/platform/rockchip/rga/rga-hw.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
index be1bc8ddbd03b..2e7ec8b7c20f7 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -158,7 +158,6 @@ static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
 
 static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 {
-	struct rockchip_rga *rga = ctx->rga;
 	u32 *dest = ctx->cmdbuf_virt;
 	unsigned int scale_dst_w, scale_dst_h;
 	unsigned int src_h, src_w, dst_h, dst_w;
@@ -251,13 +250,6 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	 */
 	if (src_info.data.rot_mode == RGA_SRC_ROT_MODE_90_DEGREE ||
 	    src_info.data.rot_mode == RGA_SRC_ROT_MODE_270_DEGREE) {
-		if (rga->version.major == 0 || rga->version.minor == 0) {
-			if (dst_w == src_h)
-				src_h -= 8;
-			if (abs(src_w - dst_h) < 16)
-				src_w -= 16;
-		}
-
 		scale_dst_h = dst_w;
 		scale_dst_w = dst_h;
 	} else {
-- 
2.55.0

Re: [PATCH v3] media: rockchip: rga: fix rotation on rev0 hardware

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]

Le mardi 29 septembre 2026 à 10:41 +0100, John Keeping a écrit :
> On RK3288 where the RGA reports version zero [1], rotating a buffer
> gives corrupt output where one edge will be truncated.  This is a result
> of some adjustment done specifically for hardware revision zero.  But it
> seems this is not necessary and there is no evidence for why this
> adjustment should be applied on these hardware revisions.
> 
> Comparing the TRMs between RK3288 with revision zero and RK3588 with
> revision 3.2 there is evidence of new features supported by later
> hardware, but nothing indicating any difference in behaviour for
> rotation which is supported by all hardware revisions.
> 
> The logic here looks wrong anyway, since it's bizarre that minor
> revision zero would need the adjustment for _all_ major revisions.  This
> was added with the initial version of the driver in commit f7e7b48e6d796
> ("[media] rockchip/rga: v4l2 m2m support") so there is no detail in the
> Git history to indicate why this special-case might be needed.
> 
> [1] Comparing with the vendor code, it seems this is actually 1.3 with
>     the hardware reporting zero more by coincidence than design since the
>     version register is not defined for RK3288.
> 
> Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>

Fixes: f7e7b48e6d796 ("[media] rockchip/rga: v4l2 m2m support")
Cc: stable@vger.kernel.org
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
> base-commit: 2579cbe68005f46fc7f8f95364f6b101b07b9d1c
> 
> v3:
> - Rebase on media-committers/fixes and add base-commit to hopefully make
>   CI happy (patch content is completely unchanged)
> v2:
> - Remove variable that is unused after this change
> ---
>  drivers/media/platform/rockchip/rga/rga-hw.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
> index be1bc8ddbd03b..2e7ec8b7c20f7 100644
> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
> @@ -158,7 +158,6 @@ static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
>  
>  static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
>  {
> -	struct rockchip_rga *rga = ctx->rga;
>  	u32 *dest = ctx->cmdbuf_virt;
>  	unsigned int scale_dst_w, scale_dst_h;
>  	unsigned int src_h, src_w, dst_h, dst_w;
> @@ -251,13 +250,6 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
>  	 */
>  	if (src_info.data.rot_mode == RGA_SRC_ROT_MODE_90_DEGREE ||
>  	    src_info.data.rot_mode == RGA_SRC_ROT_MODE_270_DEGREE) {
> -		if (rga->version.major == 0 || rga->version.minor == 0) {
> -			if (dst_w == src_h)
> -				src_h -= 8;
> -			if (abs(src_w - dst_h) < 16)
> -				src_w -= 16;
> -		}
> -
>  		scale_dst_h = dst_w;
>  		scale_dst_w = dst_h;
>  	} else {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]