Am 22.11.21 um 09:10 schrieb Sascha Hauer: > Hi Alex, > > On Mon, Nov 22, 2021 at 12:18:47AM +0100, Alex Bee wrote: >> Hi Sascha, >> >> Am 17.11.21 um 15:33 schrieb Sascha Hauer: >>> This series adds initial graphics support for the Rockchip RK356[68] >>> SoCs. Graphics support is based around the VOP2 controller which >>> replaces the VOP controller found on earlier Rockchip SoCs. The driver >>> has been tested with HDMI support included in this series and MIPI-DSI >>> which is not included because it needs some more work. The driver is >>> taken from the downstream Rockchip kernel and heavily polished, most non >>> standard features have been removed for now. I tested the driver with >>> the libdrm modetest utility and also with weston with both pixman and >>> panfrost driver support. Michael Riesch reported the driver to work on >>> the RK3566 as well, but device tree support for this SoC is not yet >>> included in this series. >>> >>> The HDMI changes are based on patches from Benjamin Gaignard, but >>> modified a bit as I found out that the HDMI port on the RK3568 only >>> needs one additional clock, not two. Also I added regulator support >>> which is needed to get the HDMI up on the rk3568-EVB board. >>> >>> All review and testing feedback welcome >> >> thanks for working on that - it's very (very,very) much appreciated. >> >> It took me some time to figure it out: It seems rk3568-iommu driver s >> broken - I did only get "white noise" when using it alongside vop >> (similar like it was reported here before). However: removing the >> iommu-property from vop makes it working for me with HDMI output on >> quartz64 as well. Could you check if you have the iommu driver in kernel >> enabled if it works for you, if the property is present in DT? (I used >> 5.16-rc1 + this series + [0]). > I have the iommu driver enabled and it works for me. I get this during > boot: > > [0.263287] rockchip-vop2 fe040000.vop: Adding to iommu group 0 > > So I expect it is indeed used. > >> Also vop mmu seems to have the >> power-domain missing in your series (same as downstream) - however >> adding that doesn't help much currently. > Probably the power domain gets enabled anyway when the VOP is activated, > so adding it to the iommu won't help anything. Nevertheless it seems > correct to add the property, I'll do so in the next round. > >> As a sidenote: I verfied this with using Ezequiel's vpu addtion for >> RK356x: It did only work when removing the iommu there as well (getting >> tons of page faults otherwise) - so iommu driver really seems to broken, >> at least for RK3566. (Or I'm a missing a option in kernel config, which >> wasn't required for the older iommu version?) > I don't think so. I started from defconfig and disabled other > architectures and unneeded drivers, but I did not enable anything > specific to iommu. I've found out now that I can make it work with iommu, by limiting the available memory to something below 4G (I have a 8G board). So there is something wrong in the driver or somewhere in memory mapping, iommu api (since it works when using CMA), ... however: it does clearly not relate to your patch. >> >> But as reported before: For HDMI this does currently only work for pixel >> clock rates, which are integer-divisable with hpll clock rate (which is >> the hardcoded parent of vop0's dclk) >> As discussed in Benjamin's initial submission of the addition of >> RK3568's hdmi controller [1] same as with RK3288's and RK3399's hdmi phy >> needs a reference clock (it's called vpll there) which needs to get >> switched before the vop switches the mode (since phy rate switching is >> done before) - it's HPLL in case of RK356x. For whatever reason it's >> called "ref" for RK356x only downstream [2] - so you should add another >> clock "vpll" (renaming it to "ref" for _ALL_ SoCs which have it would be >> a _GREAT_ idea) which is <&pmucru PLL_HPLL>. > Yeah, a consumer clock should be named after the usage in the consumer, > not after the provider name. I also stumbled over this and naming it > "ref" makes much more sense. We'll likely have to keep supporting "vpll" > as well for compatibility to old device trees. > >> What brings us to the "real" clock problem and the reason, why >> non-integer divisable pixel clock rates are not possible ATM: This is a >> long standing issue for RK3288 and RK3399 as well (and one of the main >> reasons why 4k modes are not possible for those older SoCs currently): >> Upstream all PLL rates are controlled with those PLL rate tables in the >> clock driver and they have to be _exactly_ defined as they are used >> (HDMI sinks are very picky). >> You will not see any additional rates downstream for RK3568: they have a >> mechanism there to automatically calculate the PLL settings if the rate >> doesn't exist in these tables (IIRC this was submitted upstream also: >> but it was rejected/ignored by maintainers). > Looks like we have to try harder to get it upstream. Do you have a > pointer to this patch? Sure: https://patchwork.kernel.org/project/linux-clk/patch/20191204082527.19957-1-zhangqing@xxxxxxxxxxxxxx/ I don't know if that is the was last submitted version. Best regards, Alex >> As a quick hackarround (for >> testing): You could use this table [3] we are using in LibreElec for >> RK3399 to get 4k modes working and assign it to HPLL in RK3568's clock >> driver (I tested it and it works great). It might be possible to just >> add those rates (some also without frac dividers) to the common PLL >> table for RK3568. > Thanks for noting. This could also explain why currently only 1080p is > working. > > Sascha >