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

Johan Hovold
2025-02-05T08:30:39Z

On Wed, Feb 05, 2025 at 11:47:12AM +0530, Dikshita Agarwal wrote:
> 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:

> >>>> 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.

I was only thinking about the (experimental) support for hardware
already supported by venus (i.e. initially 8250) and possibly the
module parameter if you think that is essential.

I can imagine the feature set becoming non-overlapping overtime so that
you end up with some users depending on venus and some depending on iris
for the same hardware if you expose iris support prematurely.

> 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.

Right, it would, and that is not necessarily right.

> And will this also be needed to be captured in cover letter?

Yes, whatever solution you end up with need to be described and
motivated in the cover letter (and commit messages) so that the
reasoning can be evaluated.

Johan