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:

quoted

On Mon, Feb 03, 2025 at 09:22:51AM +0100, Johan Hovold wrote:

quoted

On Fri, Jan 31, 2025 at 10:44:28AM -0800, Abhinav Kumar wrote:

quoted

quoted

quoted

Solution: If the user prefers iris driver and iris driver has not probed 
yet, and if venus tries to probe ahead of iris we keep -EDEFERing till 
iris probes and succeeds. Vice-versa when the preference is venus as well.

This sounds wrong too.

Look, first you guys need to explain *why* you want to have two drivers
for the same hardware (not just to me, in the commit message and cover
letter).

That's something that really should never be the case and would need to
be motivated properly.

I think it has been written several time (not sure about this commit):
to provide a way for a migration path _while_ people are working on Iris
features. Being able to A/B test Venus and Iris drivers and to report
possible regressions or lack of those on the common platforms supported
by those (sm8250 at this moment).

You don't need a module parameter for that.

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.

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.

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.

I'm sure you have some answers, but you need to provide them as part of
the series.

quoted

quoted

Second, if the reasons for keeping both drivers are deemed justifiable,
you need to come up with mechanism for only binding one of them.

I already told you that module parameters is not the way to go here (and
the msm drm driver's abuse of module parameters is not a good precedent
here).

If this is a transitional thing (which it must be), then just add a
Kconfig symbol to determine which driver should probe. That's good
enough for evaluating whatever needs to be evaluated, and doesn't
depend on adding anti-patterns like module parameters (and helper
modules for them).

No, Kconfig complicates A/B testing. What you usually want to do is to
boot a kernel, then go over a bunch of files testing that they work with
both drivers. Kconfig implies booting abother kernel, etc.

No, I'm pretty sure you'd even want to reboot in between so as not to
rely on state left behind by the other driver.

As a quick test I'd definitely do not want to reboot. Both drivers
completely shut down the firmware running on the core, so there is no
intermediate state left between driver probes.

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.

-- 
With best wishes
Dmitry