[PATCH v5 16/29] media: rockchip: rga: split flip and rotate into separate function

[Sven Püschel]

Hi Nicola,

On 5/12/26 4:15 PM, Nicolas Dufresne wrote:
> Le mardi 12 mai 2026 à 16:08 +0200, Sven Püschel a écrit :
>
>
>>> To stick with the spec, the capture format needs to be updated, and it needs to
>>> happen in a way user can be able to read it back for the correct frame if
>>> userspace make use of the queues. I see 3 options, let me know what you think,
>>> or what is later implemented if you already thought about that.
>>>
>>> 1. Synchronously update the capture format width/height, document in the
>>> respective control this behaviour, leaving to userspace to remember which frames
>>> the change will apply to.
>>>
>>> This works nicely for this type of HW, but would be a bit complicated for a
>>> deinterlacer, since the buffering might be HW specific. It also make usage of
>>> queues harder, less independent.
>>>
>>> 2. Force a drain/stop/start for any 90 degree rotation
>>>
>>> This might impose a longer idle time for the converter core, and is kind of
>>> opposite of your commit message. But requires no spec work.
>>>
>>> 3. Emit SRC_CH, implement the drain procedure typical to decoder resolution
>>> change.
>>>
>>> Typically it means userspace can keep buffering on the OUTPUT queue, and once
>>> the LAST buffer is met, it can simply read the new format (and new stride, since
>>> due to alignment, this might be hardware specific) and toggle streamoff/on only
>>> on capture queue to reactivate the processing.
>>>
>>> The 3. is more complex for the driver, but its a proven race-free method for
>>> decoders already. 2 would be statusquo to get this series in, and we could post-
>>> poned more advance work for seamless 90degree rorations. 1., I don't really like
>>> that solution, it not quite generic enough.
>> I'll go with option 2 for now to keep it simple and improve the status
>> quo a bit.
> Works for me.

Philipp Zabel just mentioned that a 90 degree rotation would just cause 
the RGA to scale it to the output format (deforming if it isn't 
quadratic). The existing code already considers the rotation to set the 
scaling factor accordingly (which I've also missed in this commit. But 
the commit is dropped anyways in v6 due to the various footguns).

While I see that the V4L2_CID_ROTATE docs mention the need to set the 
format according to the chosen rotation, it feels like it's intended for 
non-scaling converters. So I don't see a problem to just allow the 
current state, as the user has to adjust the format anyways if he isn't 
interested in a deformed image (instead of blocking this potential rare 
use-case).

But I'd add a check in my scaling commit to also check in the streaming 
state that we don't set a 90 degree rotation causing the scaling factor 
to be exceeded (e.g. 1x2 -> 1x32 scales by 16, whereas 90 degree 
rotation causes a scaling factor of 32).

Sincerely
     Sven

>
> Nicolas
>
>> Sincerely
>>       Sven
>>
>>> feedback welcome,
>>> Nicolas
>>>
>>>> ---
>>>>    drivers/media/platform/rockchip/rga/rga-hw.c | 57 +++++++++++++++++-----------
>>>>    1 file changed, 34 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
>>>> index dac3cb6aa17d3..6c1956b04f6ba 100644
>>>> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
>>>> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
>>>> @@ -156,7 +156,38 @@ static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
>>>>    	dest[reg >> 2] |= 0x7 << 8;
>>>>    }
>>>>    
>>>> -static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
>>>> +static void rga_cmd_set_flip_rotate_info(struct rga_ctx *ctx)
>>>> +{
>>>> +	u32 *dest = ctx->cmdbuf_virt;
>>>> +	union rga_src_info src_info;
>>>> +
>>>> +	src_info.val = dest[(RGA_SRC_INFO - RGA_MODE_BASE_REG) >> 2];
>>>> +
>>>> +	if (ctx->vflip)
>>>> +		src_info.data.mir_mode |= RGA_SRC_MIRR_MODE_X;
>>>> +
>>>> +	if (ctx->hflip)
>>>> +		src_info.data.mir_mode |= RGA_SRC_MIRR_MODE_Y;
>>>> +
>>>> +	switch (ctx->rotate) {
>>>> +	case 90:
>>>> +		src_info.data.rot_mode = RGA_SRC_ROT_MODE_90_DEGREE;
>>>> +		break;
>>>> +	case 180:
>>>> +		src_info.data.rot_mode = RGA_SRC_ROT_MODE_180_DEGREE;
>>>> +		break;
>>>> +	case 270:
>>>> +		src_info.data.rot_mode = RGA_SRC_ROT_MODE_270_DEGREE;
>>>> +		break;
>>>> +	default:
>>>> +		src_info.data.rot_mode = RGA_SRC_ROT_MODE_0_DEGREE;
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	dest[(RGA_SRC_INFO - RGA_MODE_BASE_REG) >> 2] = src_info.val;
>>>> +}
>>>> +
>>>> +static void rga_cmd_set_format_scale_info(struct rga_ctx *ctx)
>>>>    {
>>>>    	struct rockchip_rga *rga = ctx->rga;
>>>>    	u32 *dest = ctx->cmdbuf_virt;
>>>> @@ -219,27 +250,6 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
>>>>    		}
>>>>    	}
>>>>    
>>>> -	if (ctx->vflip)
>>>> -		src_info.data.mir_mode |= RGA_SRC_MIRR_MODE_X;
>>>> -
>>>> -	if (ctx->hflip)
>>>> -		src_info.data.mir_mode |= RGA_SRC_MIRR_MODE_Y;
>>>> -
>>>> -	switch (ctx->rotate) {
>>>> -	case 90:
>>>> -		src_info.data.rot_mode = RGA_SRC_ROT_MODE_90_DEGREE;
>>>> -		break;
>>>> -	case 180:
>>>> -		src_info.data.rot_mode = RGA_SRC_ROT_MODE_180_DEGREE;
>>>> -		break;
>>>> -	case 270:
>>>> -		src_info.data.rot_mode = RGA_SRC_ROT_MODE_270_DEGREE;
>>>> -		break;
>>>> -	default:
>>>> -		src_info.data.rot_mode = RGA_SRC_ROT_MODE_0_DEGREE;
>>>> -		break;
>>>> -	}
>>>> -
>>>>    	/*
>>>>    	 * Calculate the up/down scaling mode/factor.
>>>>    	 *
>>>> @@ -431,7 +441,8 @@ static void rga_cmd_set(struct rga_ctx *ctx,
>>>>    
>>>>    	rga_cmd_set_src_info(ctx, &src->offset);
>>>>    	rga_cmd_set_dst_info(ctx, &dst->offset);
>>>> -	rga_cmd_set_trans_info(ctx);
>>>> +	rga_cmd_set_format_scale_info(ctx);
>>>> +	rga_cmd_set_flip_rotate_info(ctx);
>>>>    
>>>>    	rga_write(rga, RGA_CMD_BASE, ctx->cmdbuf_phy);
>>>>