13:48 <Googulator> Not directly Panfrost-related, but since most development of pancsf is happening on RK3588, I'm asking here: what's the planned way forward with VOP2 support in mainline?

13:52 <Googulator> Right now, we have ported versions of VOP2 and its associated drivers in bbrezillon 's tree, as well as in midstream, but there's a different VOP2 driver in mainline that stands in the way.

14:01 <bbrezillon> Googulator: the VOP2 + HDMI stuff in my tree are really just hacks

14:04 <Googulator> I don't know if it's feasible to make it any less of a hack

14:05 <Googulator> (if by "hack" you mean it's not a proper continuation of the upstream version's history)

14:06 <bbrezillon> I can't tell when this will happen, but the plan is to extend existing drivers (dw-hdmi + rockchip VOP2) to support what we want to support

14:07 <Googulator> I looked into that, but it seems very problematic in the case of VOP2

14:07 <Googulator> (HDMI is doable)

14:07 <bbrezillon> by hack I mean rockchip has forked the drivers and heavily diverged from the upstream version, and what I did was just copy the drivers and get it to work on 6.something

14:08 <bbrezillon> what's the problem with VOP2?

14:08 <Googulator> Upstream VOP2 isn't merely divergent, it has no obvious common history with the downstream one

14:08 <CounterPillow> because downstream vop2 was first and then someone at pengutronix made a cleaned up upstream vop2

14:09 <Googulator> Several years ago, Rockchip upstreamed a minimal, gutted version of the original VOP driver, with only basic features kept

14:09 <Googulator> Then someone (pengutronix?) added support for RK356x VOP2 (but not RK3588) to that driver

14:09 <bbrezillon> sure, I meant, what makes it impossible to extend the upstream VOP2 driver to support the use cases supported by the downstream one?

14:10 <Googulator> The downstream driver heavily relies on those features of the common VOP1/2 driver that were cut from upstream

14:12 <bbrezillon> you shouldn't think in term of "how can I port the downstream driver to upstream kernels?", but rather "how can I port a specific feature I need to the upstream kernel, and port it in a way that make it acceptable upstream"

14:13 <CounterPillow> ^

14:14 <bbrezillon> same goes for dw-hdmi-qp, we certainly don't want to fork dw-hdmi as they did

14:15 <Googulator> My issue is, the original downstream driver, pre-gutting, would now probably be acceptable, since the formerly Android-exclusive DRM/DRI features that probably lead to a gutted version being submitted, are all now upstreamed

14:15 <CounterPillow> hahahahahaa no it wouldn't be

14:15 <bbrezillon> and for the samsung hdmi-phy, we probably want to abstract hdmi phys through the phy framework, so the dw-hdmi driver can use a phy that's not designware

14:16 <CounterPillow> 1. it doesn't use regmaps properly iirc, 2. they messed up locking, 3. they include half a clock driver in the middle of it as a C file just to do some calculations

14:17 <robmur01> dw-hdmi-rockchip has already been wanting to support external phys *properly* since RK3328...

14:17 <bbrezillon> 4. It's not using the DRM API correctly :-)

14:19 <CounterPillow> right, it was some half-atomic thing

14:20 <bbrezillon> robmur01: oh, while you're here, I gave your io-pgtble custom allocator a try. It's here, in case you want to have a look => https://gitlab.freedesktop.org/bbrezillon/linux/-/commits/panthor/

14:20 <bbrezillon> *your io-pgtable custom allocator idea...

14:21 <Googulator> (by the original downstream driver, I mean VOP/VOP2, not HDMI)

14:21 <CounterPillow> yes we understood that

14:24 <bbrezillon> Googulator: sure, I was just pointing out that porting dw-hdmi-qp is not as trivial as you might think

14:25 <bbrezillon> because we don't want it done the way it's done in the downstream tree

14:25 <bbrezillon> (or in my hacky branch)

14:25 <Googulator> For HDMI, I agree, extending the existing upstream driver is probably the way to go

14:27 <Googulator> But with VOP2, while it may be possible to extend the current upstream one to support _some_ of the RK3588's functionality, I don't see it ever reaching parity with the downstream driver

14:27 <bbrezillon> hence my original question, what makes it impossible to port downstream VOP2 features to upstream VOP2 driver?

14:28 <CounterPillow> If you think making downstream VOP2 conform to upstream standards is easier than implementing RK3588 support for upstream VOP2 then your calibration on that is way off.

14:29 <bbrezillon> the key here is to incrementally add features to upstream VOP2 so it eventually reaches the feature-set available in rockchip's downstream driver

14:29 <Googulator> The problem is, we don't even really know what's missing

14:30 <Googulator> Because it was stripped down without any history surviving

14:30 <CounterPillow> You have to read the code to port things usually, yes.

14:31 <bbrezillon> yeah, what CounterPillow said, read the code, read the TRMs, and port the features you need

14:31 <bbrezillon> not the ones you might want to support one day

14:32 <bbrezillon> of course, that doesn't mean you shouldn't plan for those advanced features

14:33 <bbrezillon> but trying to port all the features at once is the best way to get burnt

14:37 <CounterPillow> And if you submit code for upstream inclusion, you need to be able to justify the code's existence and correctness, which means you need to understand the code, at which point you can just add the feature to upstream vop2 anyway.

14:55 <robmur01> bbrezillon: looks OK at a glance, although if it were me I'd be inclined to refactor a bit harder to just pass data through to __arm_lpae_alloc_pages() instead of cfg+cookie (more consistent with previous changes)

14:56 <robmur01> undecided about the amount of sanity-checking, but we can revisit that once we get as far as proper review :)

15:31 <bbrezillon> robmur01: I tried that, and it didn't work because iop is allocated in the io-pgtable backend, but iop->{cfg,cookie,fmt} is initialized by the core in alloc_io_pgtable_ops() after the backend ->alloc() function returns

15:33 <robmur01> Oh, right, because we allocate the pgd up-front from there. Oh well. :(

15:33 <bbrezillon> I mean, I could add an extra iop->{cfg,cookie} = *cfg,cookie in the arm pgtable driver, but I wasn't sure

15:35 <bbrezillon> re sanity checking => that's mostly here to catch iommu drivers trying to pass custom allocators to a pgtable driver that doesn't support it

15:37 <bbrezillon> I guess that's just based on personal experience trying to figure out why my code doesn't work when I'm actually trying to use a feature that's not supported but the framework lets pass the feature flag/props without complaining :-)

15:38 <bbrezillon> I'll probably post those patches along with the panthor patches