linux-kernel - Re: [PATCH v2 5/7] media: rkvdec: Disable QoS for HEVC and VP9 on RK3328

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9dce97a9-92e6-4803-9e06-b2938e3c4999@kwiboo.se>
Date: Tue, 12 Aug 2025 01:08:04 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Nicolas Dufresne <nicolas.dufresne@...labora.com>
Cc: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
 Detlev Casanova <detlev.casanova@...labora.com>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Heiko Stuebner
 <heiko@...ech.de>, Alex Bee <knaerzche@...il.com>,
 Sebastian Fricke <sebastian.fricke@...labora.com>,
 linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/7] media: rkvdec: Disable QoS for HEVC and VP9 on
 RK3328

Hi Nicolas,

Missed some comments in my last mail.

On 8/11/2025 11:25 PM, Nicolas Dufresne wrote:
> Le dimanche 10 août 2025 à 21:24 +0000, Jonas Karlman a écrit :
>> From: Alex Bee <knaerzche@...il.com>
>>
>> The RK3328 VDEC has a HW quirk that require QoS to be disabled when HEVC
>> or VP9 is decoded, otherwise the decoded picture may become corrupted.
>>
>> Add a RK3328 variant with a quirk flag to disable QoS when before
>> decoding is started.
>>
>> Signed-off-by: Alex Bee <knaerzche@...il.com>
>> Signed-off-by: Jonas Karlman <jonas@...boo.se>
>> ---
>> Changes in v2:
>> - No change
>> ---
>>  drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c |  9 +++++++++
>>  drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h |  2 ++
>>  drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c  | 10 ++++++++++
>>  drivers/media/platform/rockchip/rkvdec/rkvdec.c      | 12 ++++++++++++
>>  drivers/media/platform/rockchip/rkvdec/rkvdec.h      |  4 ++++
>>  5 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>> b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>> index 1994ea24f0be..f8bb8c4264f7 100644
>> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>> @@ -789,6 +789,15 @@ static int rkvdec_hevc_run(struct rkvdec_ctx *ctx)
>>  	writel(1, rkvdec->regs + RKVDEC_REG_PREF_LUMA_CACHE_COMMAND);
>>  	writel(1, rkvdec->regs + RKVDEC_REG_PREF_CHR_CACHE_COMMAND);
>>  
>> +	if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS) {
>> +		u32 reg;
>> +
>> +		reg = readl(rkvdec->regs + RKVDEC_REG_QOS_CTRL);
>> +		reg |= 0xFFFF;
>> +		reg &= ~BIT(12);
> 
> I wonder if there is a better way to express that, if not, a comment for future
> readers would be nice. If read it will, we keep the upper 16bit, and replaced
> the lower bits with 0xEFFF (all bits set except 12) ? I'd rather not spend time
> thinking if I walk by this code again.

Vendor kernel use following comment to describe the purpose of this [1]:

  HW defeat workaround: VP9 and H.265 power save optimization cause
  decoding corruption, disable optimization here.

>From the TRM we can see following for rkvdec_swreg99_qos_ctrl:

  27:26 sw_axi_wr_hurry_level
    00: hurry off 
    01~11: hurry level 
  25:24 sw_axi_rd_hurry_level
    00: hurry off 
    01~11: hurry level 
  23:16 sw_bus2mc_buffer_qos_level
    range is: 0~255
    the value is means that left space <=
    sw_bus2mc_buffer_qos_level, it will give hurry
  15:0 swreg_block_gating_e

So yes this set swreg_block_gating_e to 0xEFFF. Possible this configure
hw to not auto gate most internal clocks?

Could add a comment and possible use something like following:

  reg &= GENMASK(31, 16);
  reg |= 0xEFFF;

[1] https://github.com/Kwiboo/linux-rockchip/blob/linux-6.1-stan-rkr6.1/drivers/video/rockchip/mpp/mpp_rkvdec.c#L857-L867

> 
>> +		writel(reg, rkvdec->regs + RKVDEC_REG_QOS_CTRL);
>> +	}
>> +
>>  	/* Start decoding! */
>>  	reg = (run.pps->flags & V4L2_HEVC_PPS_FLAG_TILES_ENABLED) ?
>>  		0 : RKVDEC_WR_DDR_ALIGN_EN;
>> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>> b/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>> index 540c8bdf24e4..c627b6b6f53a 100644
>> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>> @@ -219,6 +219,8 @@
>>  #define RKVDEC_REG_H264_ERR_E				0x134
>>  #define RKVDEC_H264_ERR_EN_HIGHBITS(x)			((x) & 0x3fffffff)
>>  
>> +#define RKVDEC_REG_QOS_CTRL				0x18C
>> +
>>  #define RKVDEC_REG_PREF_LUMA_CACHE_COMMAND		0x410
>>  #define RKVDEC_REG_PREF_CHR_CACHE_COMMAND		0x450
>>  
>> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>> b/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>> index 0e7e16f20eeb..cadb9d592308 100644
>> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>> @@ -824,6 +824,16 @@ static int rkvdec_vp9_run(struct rkvdec_ctx *ctx)
>>  	writel(1, rkvdec->regs + RKVDEC_REG_PREF_CHR_CACHE_COMMAND);
>>  
>>  	writel(0xe, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
>> +
>> +	if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS) {
>> +		u32 reg;
>> +
>> +		reg = readl(rkvdec->regs + RKVDEC_REG_QOS_CTRL);
>> +		reg |= 0xFFFF;
>> +		reg &= ~BIT(12);
>> +		writel(reg, rkvdec->regs + RKVDEC_REG_QOS_CTRL);
> 
> Can we deduplicate that ?

Guess so, any suggestion on how to best do that?

One possible way that comes to mind:

  if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS)
	rkvdec_quirk_disable_qos(rkvdec);

> 
>> +	}
>> +
>>  	/* Start decoding! */
>>  	writel(RKVDEC_INTERRUPT_DEC_E | RKVDEC_CONFIG_DEC_CLK_GATE_E |
>>  	       RKVDEC_TIMEOUT_E | RKVDEC_BUF_EMPTY_E,

[snip]