Re: [Intel-gfx] [PATCH 03/26] drm/irq: Ditch DRIVER_IRQ_SHARED

2019-01-28 Thread Emil Velikov 
On 2019/01/25, Daniel Vetter wrote:
> On Fri, Jan 25, 2019 at 02:46:55PM +, Emil Velikov wrote:
> > On Thu, 24 Jan 2019 at 16:58, Daniel Vetter  wrote:
> > >
> > > This is only used by drm_irq_install(), which is an optional helper.
> > > And the right choice is to set it for all pci devices, and not for
> > > everything else.
> > >
> > Can you please add some information (or reference) why it's the right 
> > choice?
> 
> pci devices can have a shared interrupt line. That's definitely the case
> for legacy pci, and I guess carries over to pcie. msi/msi-x interrupts
> will give you dedicated interrupts, but it doesn't really hurt to mark
> them as shared.
> 
> I guess I could rephrase to state:
> 
> "This is only used by drm_irq_install(), which is an optional helper.
> For legacy pci devices this is required (due to interrupt sharing without
> msi/msi-x), and just making this the default exactly matches the behaviour
> of all existing drivers using the drm_irq_install() helpers. In case that
> ever becomes wrong drivers can roll their own irq handling, as many
> drivers already do (for other reasons like needing a threaded interrupt
> handler, or having an entire pile of different interrupt sources)."
> 
> That better?

It's amazing. Thank you very much.

-Emil
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/26] drm/irq: Ditch DRIVER_IRQ_SHARED

2019-01-25 Thread Daniel Vetter 
On Fri, Jan 25, 2019 at 02:46:55PM +, Emil Velikov wrote:
> On Thu, 24 Jan 2019 at 16:58, Daniel Vetter  wrote:
> >
> > This is only used by drm_irq_install(), which is an optional helper.
> > And the right choice is to set it for all pci devices, and not for
> > everything else.
> >
> Can you please add some information (or reference) why it's the right choice?

pci devices can have a shared interrupt line. That's definitely the case
for legacy pci, and I guess carries over to pcie. msi/msi-x interrupts
will give you dedicated interrupts, but it doesn't really hurt to mark
them as shared.

I guess I could rephrase to state:

"This is only used by drm_irq_install(), which is an optional helper.
For legacy pci devices this is required (due to interrupt sharing without
msi/msi-x), and just making this the default exactly matches the behaviour
of all existing drivers using the drm_irq_install() helpers. In case that
ever becomes wrong drivers can roll their own irq handling, as many
drivers already do (for other reasons like needing a threaded interrupt
handler, or having an entire pile of different interrupt sources)."

That better?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/26] drm/irq: Ditch DRIVER_IRQ_SHARED

2019-01-25 Thread Emil Velikov 
On Thu, 24 Jan 2019 at 16:58, Daniel Vetter  wrote:
>
> This is only used by drm_irq_install(), which is an optional helper.
> And the right choice is to set it for all pci devices, and not for
> everything else.
>
Can you please add some information (or reference) why it's the right choice?

Thanks
Emil
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 03/26] drm/irq: Ditch DRIVER_IRQ_SHARED

2019-01-24 Thread Daniel Vetter 
This is only used by drm_irq_install(), which is an optional helper.
And the right choice is to set it for all pci devices, and not for
everything else.

Any driver with special needs should just use request_irq() directly,
and there's plenty of drivers doing that already.

Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/drm_irq.c   |  4 ++--
 drivers/gpu/drm/gma500/psb_drv.c|  3 +--
 drivers/gpu/drm/i915/i915_drv.c |  2 +-
 drivers/gpu/drm/mga/mga_drv.c   |  2 +-
 drivers/gpu/drm/qxl/qxl_drv.c   |  1 -
 drivers/gpu/drm/r128/r128_drv.c |  2 +-
 drivers/gpu/drm/radeon/radeon_drv.c |  4 +---
 drivers/gpu/drm/via/via_drv.c   |  3 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  2 +-
 drivers/staging/vboxvideo/vbox_drv.c|  3 +--
 include/drm/drm_drv.h   | 24 +++-
 12 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 22502417c18c..a1bb3773087b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1189,7 +1189,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, 
unsigned int pipe,
 static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
-   DRIVER_IRQ_SHARED | DRIVER_GEM |
+   DRIVER_GEM |
DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
.load = amdgpu_driver_load_kms,
.open = amdgpu_driver_open_kms,
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c5babb3e4752..9bd8908d5fd8 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -120,8 +120,8 @@ int drm_irq_install(struct drm_device *dev, int irq)
if (dev->driver->irq_preinstall)
dev->driver->irq_preinstall(dev);
 
-   /* Install handler */
-   if (drm_core_check_feature(dev, DRIVER_IRQ_SHARED))
+   /* PCI devices require shared interrupts. */
+   if (dev->pdev)
sh_flags = IRQF_SHARED;
 
ret = request_irq(irq, dev->driver->irq_handler,
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 7cf14aeb1c28..eefaf4daff2b 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -468,8 +468,7 @@ static const struct file_operations psb_gem_fops = {
 };
 
 static struct drm_driver driver = {
-   .driver_features = DRIVER_IRQ_SHARED | \
-  DRIVER_MODESET | DRIVER_GEM,
+   .driver_features = DRIVER_MODESET | DRIVER_GEM,
.load = psb_driver_load,
.unload = psb_driver_unload,
.lastclose = drm_fb_helper_lastclose,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 550cfb945942..a7aaa1ac4c99 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -3010,7 +3010,7 @@ static struct drm_driver driver = {
 * deal with them for Intel hardware.
 */
.driver_features =
-   DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
+   DRIVER_GEM | DRIVER_PRIME |
DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ,
.release = i915_driver_release,
.open = i915_driver_open,
diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
index 1aad27813c23..6e1d1054ad06 100644
--- a/drivers/gpu/drm/mga/mga_drv.c
+++ b/drivers/gpu/drm/mga/mga_drv.c
@@ -57,7 +57,7 @@ static const struct file_operations mga_driver_fops = {
 static struct drm_driver driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY |
-   DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
+   DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ,
.dev_priv_size = sizeof(drm_mga_buf_priv_t),
.load = mga_driver_load,
.unload = mga_driver_unload,
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 44daac129205..d856615bdb50 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -243,7 +243,6 @@ static struct pci_driver qxl_pci_driver = {
 
 static struct drm_driver qxl_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
-  DRIVER_IRQ_SHARED |
   DRIVER_ATOMIC,
 
.dumb_create = qxl_mode_dumb_create,
diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
index 0d2b7e42b3a7..4b1a505ab353 100644
--- a/drivers/gpu/drm/r128/r128_drv.c
+++ b/drivers/gpu/drm/r128/r128_drv.c
@@ -57,7 +57,7 @@ static const struct file_operations r128_driver_fops = {
 static struct drm_driver driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY |
-   DRIVER_HAVE_DMA | DRIVER_