linux-kernel - Re: [PATCH v1 2/4] media: qcom: iris: Add rotation support for encoder

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c18043c0-253a-4681-8ade-659b8652a90e@oss.qualcomm.com>
Date: Fri, 17 Oct 2025 13:54:41 +0800
From: Wangao Wang <wangao.wang@....qualcomm.com>
To: Bryan O'Donoghue <bod@...nel.org>, vikash.garodia@....qualcomm.com,
        dikshita.agarwal@....qualcomm.com, abhinav.kumar@...ux.dev,
        mchehab@...nel.org
Cc: linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, quic_qiweil@...cinc.com,
        quic_renjiang@...cinc.com, Wangao Wang <wangao.wang@....qualcomm.com>
Subject: Re: [PATCH v1 2/4] media: qcom: iris: Add rotation support for
 encoder

On 10/16/2025 12:08 AM, Bryan O'Donoghue wrote:
>> iris_hfi_gen2_set_bitstream_resolution(struct iris_inst *inst, u32 pl
>>           payload_type = HFI_PAYLOAD_U32;
>>       } else {
>>           codec_align = inst->codec == V4L2_PIX_FMT_HEVC ? 32 : 16;
>> -        resolution = ALIGN(inst->enc_bitstream_width, codec_align) << 
>> 16 |
>> -            ALIGN(inst->enc_bitstream_height, codec_align);
>> +        if (is_rotation_90_or_270(inst))
>> +            resolution = ALIGN(inst->enc_bitstream_height, 
>> codec_align) << 16 |
>> +                ALIGN(inst->enc_bitstream_width, codec_align);
>> +        else
>> +            resolution = ALIGN(inst->enc_bitstream_width, 
>> codec_align) << 16 |
>> +                ALIGN(inst->enc_bitstream_height, codec_align);
> 
> That complex assignement can be expressed as a macro, static inline or 
> just a regular method instead of copy/paste the same code with one 
> variable changed.
> 
You're right, this code is reused in several places. I will address it 
in v2.>>   inline bool is_scaling_enabled(struct iris_inst *inst)
>>   {
>> -    return inst->crop.left != inst->compose.left ||
>> -        inst->crop.top != inst->compose.top ||
>> -        inst->crop.width != inst->compose.width ||
>> -        inst->crop.height != inst->compose.height;
>> +    return inst->fmt_dst->fmt.pix_mp.width != inst->fmt_src- 
>> >fmt.pix_mp.width ||
>> +        inst->fmt_dst->fmt.pix_mp.height != inst->fmt_src- 
>> >fmt.pix_mp.height;
> 
> These long chains of indirection make the code not very readable
> 
> Please take pointers to &fmt_dts->fmt and &fmt_src->fmt and reduce this 
> clause.
> 
Sure, will update in v2.

-- 
Best Regards,
Wangao