Re: [PATCH 00/16] IOMMU driver for Kirin 960/970

[Greg Kroah-Hartman]

On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> Hi Christoph,
> 
> Em Mon, 17 Aug 2020 09:21:06 +0100
> Christoph Hellwig <hch@xxxxxxxxxxxxx> escreveu:
> 
> > On Mon, Aug 17, 2020 at 09:49:59AM +0200, Mauro Carvalho Chehab wrote:
> > > Add a driver for the Kirin 960/970 iommu.
> > > 
> > > As on the past series, this starts from the original 4.9 driver from
> > > the 96boards tree:
> > > 
> > > 	https://github.com/96boards-hikey/linux/tree/hikey970-v4.9
> > > 
> > > The remaining patches add SPDX headers and make it build and run with
> > > the upstream Kernel.  
> > 
> > Please don't add iommu drivers to staging, and just work with the
> > maintainers to properly clean it up.
> 
> I need to start from the original patch in order to preserve its
> authorship.
> 
> My plan is to work with the iommu subsystem maintainers after
> have this (and another pending patch series for DRM) merged.
> 
> > I also don't think adding a totally out of date not compiling version
> > is a good idea.  Please do a proper rollup, and if required (probably
> > not in this case), split it into useful chunks.
> 
> This series make this driver working as expected.
> 
> I mean, while patch 01/16 is against Kernel 4.9, the other patches
> on this series ports it to upstream, cleans up the driver and
> address several issues on it.
> 
> This specific IOMMU seems to be an specific part of the SoC dedicated for 
> the display engine and by the encoding/decoding images via the ISP. 
> With this series, this driver builds and runs as expected, providing
> IOMMU support needed by the upcoming KMS/DRM driver.
> 
> The only issue on it (as far as I can tell) is that the DT bindings
> require some work, as, instead of using dma-ranges, the DRM driver binds
> into it with:
> 
> 	smmu_lpae {
>                  compatible = "hisilicon,smmu-lpae";
>          };
> 
>          dpe: dpe@e8600000 {
>                  compatible = "hisilicon,kirin970-dpe";
> ...
>                  iommu_info {
>                          start-addr = <0x8000>;
>                          size = <0xbfff8000>;
>                  };
>          };
> 
> In order to properly address it, the best would be to also have the
> DRM driver merged upstream, as it relies on it. So, a change in DT will 
> also mean a change at the way the DRM uses it.
> 
> The DRM itself should go via staging, as it has some bugs that I'd
> like to fix before moving it to drivers/gpu/drm. Those are more
> tricky to solve, as they seem to require using different settings for 
> some hardware registers, and the downstream driver also have the same 
> issues. Fixing them will likely require some time.

DRM drivers can't go through staging unless you get the DRM developers
to agree with it, and last I heard, they were strongly against it.

It's _always_ faster to just do the work out-of-tree for a week or so
and then merge it correctly to the proper part of the kernel tree.  I'd
recommend doing that here for the iommu driver, as well as the DRM
driver.

There's no issues with authorship and the like, just properly attribute
it when you submit it and you are fine.

Again, merging in staging always takes more work and energy, don't do it
unless there is no other way.

thanks,

greg k-h