Re: [RFC PATCH v10 1/2] media: iris: introduce helper module to select video driver

Dikshita Agarwal
2025-02-05T06:17:34Z

On 2/5/2025 4:39 AM, Dmitry Baryshkov wrote:
> On Tue, Feb 04, 2025 at 05:00:31PM +0100, Johan Hovold wrote:
>> On Tue, Feb 04, 2025 at 04:55:58PM +0200, Dmitry Baryshkov wrote:
>>> On Tue, Feb 04, 2025 at 10:31:49AM +0100, Johan Hovold wrote:
>>>> On Mon, Feb 03, 2025 at 05:16:50PM +0200, Dmitry Baryshkov wrote:
>>>>> On Mon, Feb 03, 2025 at 09:22:51AM +0100, Johan Hovold wrote:
>>>>>> On Fri, Jan 31, 2025 at 10:44:28AM -0800, Abhinav Kumar wrote:
>>
>>>> And we're still waiting to hear the answers to all of Hans' questions. I
>>>> still haven't seen anyone explaining why any of this is needed. You
>>>> could have just continued letting Venus support 8250 so presumably there
>>>> is some benefit in using Iris instead. Which? And is that potential
>>>> benefit important enough to not just wait until Iris is up to par
>>>> feature wise?
>>>
>>> Because that's exactly opposite of what Iris developers are trying to
>>> do: SM8250 and SM8550 belong to two different generations of the FW
>>> interface. By supporting both of them in the Iris driver the developers
>>> can verify that the internal driver abstractions are good enough. It has
>>> been discussed in one of the threads (or in telcos) related to the first
>>> iterations of this driver. We definitely don't want to end up in Venus
>>> situation, where the abstractions were added afterwards and in some
>>> cases they are not the best ones.
>>
>> Ok, but as I've said a number of times already, information like this
>> needs to be included in the cover letter and commit message of what is
>> posted to the list.
>>
>> Maintainers and reviewers obviously have no idea what you discussed in
>> some internal teleconference and can't be expected to remember or dig
>> this out from some old email thread either.
>>
>>> The plan is to use sm8250 as a "bridge" between two drivers, verifying
>>> that they are on par during development, then drop sm8250 from Venus
>>> driver. Then move sc7280 support too, drop all HFD_VERSION_6XX support
>>> from Venus, cleanup the remnants.
>>
>> Ok, but venus would still remain for some older hardware? It's just the
>> "hfi gen1" ones that would move to the iris eventually?
>
> Yes. At least for the foreseable future. Nobody has explored an option
> of moving older hardware to the Iris driver.
>
>>
>>> I think most of that information should have been a part of the main
>>> Iris series. If it is not, please comment there, so that those commit
>>> messages can be updated.
>>
>> Unfortunately it was not, which I also pointed in my comments to the
>> Iris series.
>>
>>>> I'm sure you have some answers, but you need to provide them as part of
>>>> the series.
>>
>>>> Unbinding and rebinding drivers is not part of any normal work flow
>>>> expect possibly during development. And a developer can easily compare
>>>> Venus and Iris for 8250 without a module parameter too.
>>>
>>> Yes, we are talking about development. And yes, modparam helps. If you'd
>>> like to do two separate kernel builds, that's fine.
>>
>> Please just motivate why you think this is needed as part of the
>> submission. And make sure that the implementation is sane (e.g. not some
>> random probe defer indefinitely thing).
>>
>> Like I said, having two drivers for the same hardware is normally not
>> something that is acceptable, and this would need to be a transitional
>> thing as we both agree. One way to guarantee that is to not expose it to
>> regular users until it is ready (e.g. a Kconfig hidden behind
>> CONFIG_EXPERT or similar). Otherwise, I fear you'll end up supporting
>> both forever (with at least one of them bitrotting behind that module
>> parameter over time).
>
> I think I'm fine with hiding IRIS behind CONFIG_EXPERT, might be a good
> idea.
Are you suggesting to add a dependency on CONFIG_EXPERT for IRIS driver?
Something like:
config VIDEO_QCOM_IRIS
tristate "Qualcomm iris V4L2 decoder driver"
depends on EXPERT

This will impact the enablement of iris driver on SM8550 as well.
And will this also be needed to be captured in cover letter?

Thanks,
Dikshita
>