Message-ID: <5ad418d9-8199-43c9-a477-1e3b939c054c@kernel.org>
Date: Wed, 2 Jul 2025 13:52:22 +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: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
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