Re: [PATCH v8 0/3] media: venus: enable venus on qcs615

From: Dmitry Baryshkov
Date: Fri Jun 06 2025 - 22:13:31 EST

On 06/06/2025 15:53, Bryan O'Donoghue wrote:
On 06/06/2025 14:32, Renjiang Han wrote:

On 6/6/2025 8:56 PM, Krzysztof Kozlowski wrote:
On 06/06/2025 14:51, Renjiang Han wrote:
On 6/6/2025 8:44 PM, Krzysztof Kozlowski wrote:
On 06/06/2025 14:37, Renjiang Han wrote:
On 6/5/2025 8:34 PM, Bryan O'Donoghue wrote:
On 31/05/2025 01:05, Renjiang Han wrote:
Note:
This series consist of DT patches and a venus driver patch. The patch
1/3, which is venus driver patch, can be picked independently without
having any functional dependency. But patch 2/3 & patch 3/3, which are
DT patches, still depend on [1].
I'd say 2/3 and 3/3 still depend on 1/3, otherwise we can get video
core
on QCS615 over(?)clocked.
Agree, so we need to make sure that the driver patch is not picked
after the DT patch.
This statement is confusing.

1/3 states that there will be a fallback if there is no OPP table
present.

Giving the code a glance, I believe that is so, freq_table should be
used if there is no OPP specified in the DT.

I think we are having a hard time here understanding what you are saying.

My understanding:

- venus modification is standalone 1/3
    Qcs615 will fallback if no OPP is present

- dt modification 2/3 3/3 is therefore also independent of driver

---
bod
yes, let me re-spin this with driver patch alone. Once that gets in,
will bring in the DT patches.
Did you read my feedback? There is no "once that gets in". DTS is an
independent hardware description and your patchset claiming there is
dependency is just broken.

I am repeating this since few emails, so shall I NAK it that you will
address the main issue you have?

Best regards,
Krzysztof
Hi Krzysztof

SC7180 and QCS615 use the same video core. Only difference lies in the
freq_table for the video. Freq_table is generally determined at SOC level.
The Venus driver does not currently handle freq_table compatibility well
across platforms. This patch enables the driver to use the OPP-table from
the DT, addressing the frequency compatibility issue.
This does not resolve the main problem at all. If SW cannot use the
fallback alone, your fallback has no meaning and is not only confusing
but actually incorrect. And based on previous statements like
"overclocking" it is not only incorrect, but even harmful.

Best regards,
Krzysztof
The fallback is only triggered when there is no OPP table in the DT.
Since the QCS615 DT will include an OPP table, the fallback logic will
not be used.

Also, if the freq from the freq_table and the OPP table are the same,
would it be acceptable to drop the freq_table from the driver?

No, it's not acceptable, because then you'll break support for old DTs, which is a no-go.


If you drop the freq_table, you will need to apply OPPs for the sc7180 to DTS first before venus or you'll break sc7180.

I think TBH you should add a freq_tbl for QCS615 and make it so the order of patch application doesn't matter wrt adding OPP support.

That would require adding board data for QCS615, which definitely doesn't look like a good solution. It is _exactly_ the same as SC7180. I'm against duplicating it just for the sake of having freq_tbl.


- Add QCS freq_tbl
- Add OPP support

Then do whatever in DTS, nothing can break in this case.

As we've established the fallback isn't a fallback because it falls back to wrong data, so lets fix that.

Why isn't it a fallback? With the driver changes in place, the fallback is totally correct.

--
With best wishes
Dmitry