Message-ID: <5a5bb85f-84de-4e3a-83ff-4516706b7ad4@kernel.org>
Date: Wed, 2 Jul 2025 13:54:07 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Vikash Garodia <quic_vgarodia@...cinc.com>,
 Dikshita Agarwal <quic_dikshita@...cinc.com>,
 Abhinav Kumar <abhinav.kumar@...ux.dev>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 0/5] Introduce "non-pixel" sub node within iris video
 node

On 02/07/2025 13:52, Krzysztof Kozlowski wrote:
> On 02/07/2025 13:37, Vikash Garodia wrote:
>>
>> On 7/2/2025 4:48 PM, Krzysztof Kozlowski wrote:
>>> On 27/06/2025 17:48, Vikash Garodia wrote:
>>>> This series introduces a sub node "non-pixel" within iris video node.
>>>> Video driver registers this sub node as a platform device and configure 
>>>> it for DMA operations. All non pixel buffers, i.e bitstream, HFI queues 
>>>> and internal buffers related to bitstream processing, would be managed 
>>>> by this non_pixel device.
>>>>
>>>> Purpose to add this sub-node:
>>>> Iris device limits the IOVA to an addressable range of 4GiB, and even 
>>>> within that range, some of the space is used by IO registers, thereby 
>>>> limiting the available IOVA to even lesser. For certain video usecase, 
>>>> this limited range in not sufficient enough, hence it brings the need to 
>>>> extend the possibility of higher IOVA range.
>>>>
>>>> Video hardware is designed to emit different stream-ID for pixel and 
>>>> non-pixel buffers, thereby introduce a non-pixel sub node to handle 
>>>> non-pixel stream-ID into a separate platform device.
>>>> With this, both iris and non-pixel device can have IOVA range of 
>>>> approximately 0-4GiB individually for each device, thereby doubling the 
>>>> range of addressable IOVA.
>>>>
>>>> Tested on SM8550 and SA8775p hardwares.
>>>>
>>>> Signed-off-by: Vikash Garodia <quic_vgarodia@...cinc.com>
>>>> ---
>>>> Changes in v3:
>>>> - Add info about change in iommus binding (Thanks Krzysztof)
>>>
>>> Nothing improved in commit msg. You are changing existing device and the
>>> reason for that change is not communicated at all.
>>>
>>> There was big feedback from qualcomm saying that some commit in the past
>>> received review, so future commits can repeat the same stuff. If qcom
>>> approaches that way, sorry, no you need to come with proper commit
>>> description.
>>>
>>> Please align internally how to solve it, because my response that past
>>> imperfect review is not justification for whatever future issues was not
>>> enough.
>> Sure, lets take this as an example and you can suggest to provide a better
>> commit message for this case, it would help me to compare where is the gap. I
>> have tried my best to capture and explain the limitations and how the changes
>> address those limitations. If that is not sufficient, we might have the perfect
>> message from you and compare to find the gaps and improve, I am sorry, but thats
> 
> It is not question to me: I did not want imperfectness. Qualcomm

Double negation... should be:

"It is not question to me: I did not want perfectness. Qualcomm"

> engineer used issues in existing commits or imperfect commit in
> discussion, so that's my solution. I don't need that perfect commit, but
> it seems if I agree to that, then I will have to defend it later. Well,
> no, I don't want it.
> 
>> how i feel at the moment.
> Sure, I feel confused now as well.
> 
> Anyway, in other messages I explained what is missing. You are changing
> existing hardware and you clearly must explain how existing hardware is
> affected, how can we reproduce it, how users are affected.
> 
> All these answers.



Best regards,
Krzysztof

Powered by blists - more mailing lists