Hi,

On Thu, Feb 25, 2021 at 07:01:57PM +0000, Sudip Mukherjee wrote:
> On Tue, Feb 23, 2021 at 02:09:58PM +0100, Greg KH wrote:
> > On Tue, Feb 23, 2021 at 01:51:09PM +0100, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 23.02.21 um 13:24 schrieb Greg KH:
> > > > On Tue, Feb 23, 2021 at 01:14:30PM +0100, Daniel Vetter wrote:
> > > > > On Tue, Feb 23, 2021 at 1:02 PM Greg KH <gre...@linuxfoundation.org> 
> > > > > wrote:
> > > > > > 
> > > > > > On Tue, Feb 23, 2021 at 12:46:20PM +0100, Daniel Vetter wrote:
> > > > > > > On Tue, Feb 23, 2021 at 12:19:56PM +0100, Greg KH wrote:
> > > > > > > > On Tue, Feb 23, 2021 at 11:58:42AM +0100, Thomas Zimmermann 
> > > > > > > > wrote:
> > > > > > > > > USB devices cannot perform DMA and hence have no dma_mask set 
> > > > > > > > > in their
> > > > > > > > > device structure. Importing dmabuf into a USB-based driver 
> > > > > > > > > fails, which
> > > > > > > > > break joining and mirroring of display in X11.
> > > > > > > > > 
> > > > > > > > > For USB devices, pick the associated USB controller as 
> > > > > > > > > attachment device,
> > > > > > > > > so that it can perform DMA. If the DMa controller does not 
> > > > > > > > > support DMA
> > > > > > > > > transfers, we're aout of luck and cannot import.
> > > > > > > > > 
> > > > > > > > > Drivers should use DRM_GEM_SHMEM_DROVER_OPS_USB to initialize 
> > > > > > > > > their
> > > > > > > > > instance of struct drm_driver.
> > > > > > > > > 
> > > > > > > > > Tested by joining/mirroring displays of udl and radeon un der 
> > > > > > > > > Gnome/X11.
> > > > > > > > > 
> > > > > > > > > v3:
> > > > > > > > >    * drop gem_create_object
> > > > > > > > >    * use DMA mask of USB controller, if any (Daniel, 
> > > > > > > > > Christian, Noralf)
> > > > > > > > > v2:
> > > > > > > > >    * move fix to importer side (Christian, Daniel)
> > > > > > > > >    * update SHMEM and CMA helpers for new PRIME callbacks
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> > > > > > > > > Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for 
> > > > > > > > > USB devices")
> > > > > > > > > Cc: Christoph Hellwig <h...@lst.de>
> > > > > > > > > Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> > > > > > > > > Cc: Johan Hovold <jo...@kernel.org>
> > > > > > > > > Cc: Alan Stern <st...@rowland.harvard.edu>
> > > > > > > > > Cc: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> > > > > > > > > Cc: Sebastian Andrzej Siewior <bige...@linutronix.de>
> > > > > > > > > Cc: Mathias Nyman <mathias.ny...@linux.intel.com>
> > > > > > > > > Cc: Oliver Neukum <oneu...@suse.com>
> > > > > > > > > Cc: Thomas Gleixner <t...@linutronix.de>
> > > > > > > > > Cc: <sta...@vger.kernel.org> # v5.10+
> > > > > > > > > ---
> <snip>
> > > > > > > > 
> > > > > > > > There shouldn't be anything "special" about a DRM driver that 
> > > > > > > > needs this
> > > > > > > > vs. any other driver that might want to know about DMA things 
> > > > > > > > related to
> > > > > > > > a specific USB device.  Why isn't this an issue with the 
> > > > > > > > existing
> > > > > > > > storage or v4l USB devices?
> > > > > > > 
> > > > > > > The trouble is that this is a regression fix for 5.9, because the 
> > > > > > > dma-api
> > > > > > > got more opinionated about what it allows. The proper fix is a 
> > > > > > > lot more
> > > > > > > invasive (we essentially need to rework the drm_prime.c to allow 
> > > > > > > dma-buf
> > > > > > > importing for just cpu access), and that's a ton more invasive 
> > > > > > > than just a
> > > > > > > small patch with can stuff into stable kernels.
> > > > > > > 
> > > > > > > This here is ugly, but it should at least get rid of black 
> > > > > > > screens again.
> > > > > > > 
> > > > > > > I think solid FIXME comment explaining the situation would be 
> > > > > > > good.
> > > > > > 
> > > > > > Why can't I take a USB patch for a regression fix?  Is drm somehow
> > > > > > stand-alone that you make changes here that should belong in other
> > > > > > subsystems?
> > > > > > 
> > > > > > {hint, it shouldn't be}
> > > > > > 
> > > > > > When you start poking in the internals of usb controller structures,
> > > > > > that logic belongs in the USB core for all drivers to use, not in a
> > > > > > random tiny subsystem where no USB developer will ever notice it?  
> > > > > > :)
> > > > > 
> > > > > Because the usb fix isn't the right fix here, it's just the duct-tape.
> > > > > We don't want to dig around in these internals, it's just a convenient
> > > > > way to shut up the dma-api until drm has sorted out its confusion.
> > > > > 
> > > > > We can polish the turd if you want, but the thing is, it's still a 
> > > > > turd ...
> > > > > 
> > > > > The right fix is to change drm_prime.c code to not call dma_map_sg
> > > > > when we don't need it. The problem is that roughly 3 layers of code
> > > > > (drm_prime, dma-buf, gem shmem helpers) are involved. Plus, since
> > > > > drm_prime is shared by all drm drivers, all other drm drivers are
> > > > > impacted too. We're not going to be able to cc: stable that kind of
> > > > > stuff. Thomas actually started with that series, until I pointed out
> > > > > how bad things really are.
> > > > > 
> > > > > And before you ask: The dma-api change makes sense, and dma-api vs drm
> > > > > relations are strained since years, so we're not going ask for some
> > > > > hack there for our abuse to paper over the regression. I've been in
> > > > > way too many of those threads, only result is shouting and failed
> > > > > anger management.
> > > > 
> > > > Let's do it right.  If this is a regression from 5.9, it isn't a huge
> > > > one as that kernel was released last October.  I don't like to see this
> > > > messing around with USB internals in non-USB-core code please.
> > > 
> > > I get
> > > 
> > >  > git tag --contains 6eb0233ec2d0
> > >  ...
> > >  v5.10-rc1
> > >  ...
> > 
> > Ah, I thought you said 5.9 was when the problem happened, ok, yes, 5.10
> > is slow to get out to a lot of distros that do not update frequently :(
> 
> iiuc, Debian Bullseye release will be having v5.10.y.
> 
> Ben ?

That is correct, Debian bullseye will ship with 5.10.y.

Regards,
Salvatore
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to