| Message ID | 20230926192054.1359127-1-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/i915/display: Created exclusive version of vga decode setup | expand |
On Wed, Sep 27, 2023 at 12:50:54AM +0530, Uma Shankar wrote: > Some of the VGA functionality is not needed by the proposed > Intel Xe driver, I wouldn't put it that way. IIRC the main issue is that X becomes a slideshow if it thinks there are multiple GPUs that have VGA decoding enabled as it insists on adjusting the VGA routing pretty much for every little operation involving any of the GPUs. The fact that our current VGA arbiter implementation is just smoke and mirrors would cause real problems if anyone actually needs to talk another GPU using legacy VGA resources. > while this will be utilized by i915. > Created a version of the function to be used exclusively by i915. > Xe will implement it's own respective version. > > v2: Addressed Jani Nikula's review comments. > > v3: Dropped a duplicate function (Jani) > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/display/intel_vga.c | 18 +----------------- > drivers/gpu/drm/i915/soc/intel_gmch.c | 14 ++++++++++++++ > drivers/gpu/drm/i915/soc/intel_gmch.h | 2 ++ > 3 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c > index 286a0bdd28c6..4b98833bfa8c 100644 > --- a/drivers/gpu/drm/i915/display/intel_vga.c > +++ b/drivers/gpu/drm/i915/display/intel_vga.c > @@ -3,11 +3,9 @@ > * Copyright © 2019 Intel Corporation > */ > > -#include <linux/pci.h> > #include <linux/vgaarb.h> > > #include <video/vga.h> > - > #include "soc/intel_gmch.h" > > #include "i915_drv.h" > @@ -99,20 +97,6 @@ void intel_vga_reset_io_mem(struct drm_i915_private *i915) > vga_put(pdev, VGA_RSRC_LEGACY_IO); > } > > -static unsigned int > -intel_vga_set_decode(struct pci_dev *pdev, bool enable_decode) > -{ > - struct drm_i915_private *i915 = pdev_to_i915(pdev); > - > - intel_gmch_vga_set_state(i915, enable_decode); > - > - if (enable_decode) > - return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | > - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; > - else > - return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; > -} > - > int intel_vga_register(struct drm_i915_private *i915) > { > > @@ -127,7 +111,7 @@ int intel_vga_register(struct drm_i915_private *i915) > * then we do not take part in VGA arbitration and the > * vga_client_register() fails with -ENODEV. > */ > - ret = vga_client_register(pdev, intel_vga_set_decode); > + ret = vga_client_register(pdev, intel_gmch_vga_set_decode); > if (ret && ret != -ENODEV) > return ret; > > diff --git a/drivers/gpu/drm/i915/soc/intel_gmch.c b/drivers/gpu/drm/i915/soc/intel_gmch.c > index 49c7fb16e934..f32e9f78770a 100644 > --- a/drivers/gpu/drm/i915/soc/intel_gmch.c > +++ b/drivers/gpu/drm/i915/soc/intel_gmch.c > @@ -5,6 +5,7 @@ > > #include <linux/pci.h> > #include <linux/pnp.h> > +#include <linux/vgaarb.h> > > #include <drm/drm_managed.h> > #include <drm/i915_drm.h> > @@ -167,3 +168,16 @@ int intel_gmch_vga_set_state(struct drm_i915_private *i915, bool enable_decode) > > return 0; > } > + > +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev, bool enable_decode) > +{ > + struct drm_i915_private *i915 = pdev_to_i915(pdev); > + > + intel_gmch_vga_set_state(i915, enable_decode); > + > + if (enable_decode) > + return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | > + VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; > + else > + return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; > +} > diff --git a/drivers/gpu/drm/i915/soc/intel_gmch.h b/drivers/gpu/drm/i915/soc/intel_gmch.h > index d0133eedc720..23be2d113afd 100644 > --- a/drivers/gpu/drm/i915/soc/intel_gmch.h > +++ b/drivers/gpu/drm/i915/soc/intel_gmch.h > @@ -8,11 +8,13 @@ > > #include <linux/types.h> > > +struct pci_dev; > struct drm_i915_private; > > int intel_gmch_bridge_setup(struct drm_i915_private *i915); > void intel_gmch_bar_setup(struct drm_i915_private *i915); > void intel_gmch_bar_teardown(struct drm_i915_private *i915); > int intel_gmch_vga_set_state(struct drm_i915_private *i915, bool enable_decode); > +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev, bool enable_decode); > > #endif /* __INTEL_GMCH_H__ */ > -- > 2.42.0
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Wednesday, September 27, 2023 7:48 PM > To: Shankar, Uma <uma.shankar@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com> > Subject: Re: [Intel-gfx] [v3] drm/i915/display: Created exclusive version of vga > decode setup > > On Wed, Sep 27, 2023 at 12:50:54AM +0530, Uma Shankar wrote: > > Some of the VGA functionality is not needed by the proposed Intel Xe > > driver, > > I wouldn't put it that way. IIRC the main issue is that X becomes a slideshow if it > thinks there are multiple GPUs that have VGA decoding enabled as it insists on > adjusting the VGA routing pretty much for every little operation involving any of > the GPUs. > > The fact that our current VGA arbiter implementation is just smoke and mirrors > would cause real problems if anyone actually needs to talk another GPU using > legacy VGA resources. Thanks Ville. I will re-word the explanation. Also, this will surely require some cleanup on the existing implementation on i915. Will create a task internally to have this tracked. For xe readiness, idea is to just refactor this and not implement it for xe. We can do it cleanly directly in upstream once xe gets part of the tree. Regards, Uma Shankar > > while this will be utilized by i915. > > Created a version of the function to be used exclusively by i915. > > Xe will implement it's own respective version. > > > > v2: Addressed Jani Nikula's review comments. > > > > v3: Dropped a duplicate function (Jani) > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_vga.c | 18 +----------------- > > drivers/gpu/drm/i915/soc/intel_gmch.c | 14 ++++++++++++++ > > drivers/gpu/drm/i915/soc/intel_gmch.h | 2 ++ > > 3 files changed, 17 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_vga.c > > b/drivers/gpu/drm/i915/display/intel_vga.c > > index 286a0bdd28c6..4b98833bfa8c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vga.c > > +++ b/drivers/gpu/drm/i915/display/intel_vga.c > > @@ -3,11 +3,9 @@ > > * Copyright © 2019 Intel Corporation > > */ > > > > -#include <linux/pci.h> > > #include <linux/vgaarb.h> > > > > #include <video/vga.h> > > - > > #include "soc/intel_gmch.h" > > > > #include "i915_drv.h" > > @@ -99,20 +97,6 @@ void intel_vga_reset_io_mem(struct drm_i915_private > *i915) > > vga_put(pdev, VGA_RSRC_LEGACY_IO); > > } > > > > -static unsigned int > > -intel_vga_set_decode(struct pci_dev *pdev, bool enable_decode) -{ > > - struct drm_i915_private *i915 = pdev_to_i915(pdev); > > - > > - intel_gmch_vga_set_state(i915, enable_decode); > > - > > - if (enable_decode) > > - return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | > > - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; > > - else > > - return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; > > -} > > - > > int intel_vga_register(struct drm_i915_private *i915) { > > > > @@ -127,7 +111,7 @@ int intel_vga_register(struct drm_i915_private *i915) > > * then we do not take part in VGA arbitration and the > > * vga_client_register() fails with -ENODEV. > > */ > > - ret = vga_client_register(pdev, intel_vga_set_decode); > > + ret = vga_client_register(pdev, intel_gmch_vga_set_decode); > > if (ret && ret != -ENODEV) > > return ret; > > > > diff --git a/drivers/gpu/drm/i915/soc/intel_gmch.c > > b/drivers/gpu/drm/i915/soc/intel_gmch.c > > index 49c7fb16e934..f32e9f78770a 100644 > > --- a/drivers/gpu/drm/i915/soc/intel_gmch.c > > +++ b/drivers/gpu/drm/i915/soc/intel_gmch.c > > @@ -5,6 +5,7 @@ > > > > #include <linux/pci.h> > > #include <linux/pnp.h> > > +#include <linux/vgaarb.h> > > > > #include <drm/drm_managed.h> > > #include <drm/i915_drm.h> > > @@ -167,3 +168,16 @@ int intel_gmch_vga_set_state(struct > > drm_i915_private *i915, bool enable_decode) > > > > return 0; > > } > > + > > +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev, bool > > +enable_decode) { > > + struct drm_i915_private *i915 = pdev_to_i915(pdev); > > + > > + intel_gmch_vga_set_state(i915, enable_decode); > > + > > + if (enable_decode) > > + return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | > > + VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; > > + else > > + return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; } > > diff --git a/drivers/gpu/drm/i915/soc/intel_gmch.h > > b/drivers/gpu/drm/i915/soc/intel_gmch.h > > index d0133eedc720..23be2d113afd 100644 > > --- a/drivers/gpu/drm/i915/soc/intel_gmch.h > > +++ b/drivers/gpu/drm/i915/soc/intel_gmch.h > > @@ -8,11 +8,13 @@ > > > > #include <linux/types.h> > > > > +struct pci_dev; > > struct drm_i915_private; > > > > int intel_gmch_bridge_setup(struct drm_i915_private *i915); void > > intel_gmch_bar_setup(struct drm_i915_private *i915); void > > intel_gmch_bar_teardown(struct drm_i915_private *i915); int > > intel_gmch_vga_set_state(struct drm_i915_private *i915, bool > > enable_decode); > > +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev, bool > > +enable_decode); > > > > #endif /* __INTEL_GMCH_H__ */ > > -- > > 2.42.0 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c index 286a0bdd28c6..4b98833bfa8c 100644 --- a/drivers/gpu/drm/i915/display/intel_vga.c +++ b/drivers/gpu/drm/i915/display/intel_vga.c @@ -3,11 +3,9 @@ * Copyright © 2019 Intel Corporation */ -#include <linux/pci.h> #include <linux/vgaarb.h> #include <video/vga.h> - #include "soc/intel_gmch.h" #include "i915_drv.h" @@ -99,20 +97,6 @@ void intel_vga_reset_io_mem(struct drm_i915_private *i915) vga_put(pdev, VGA_RSRC_LEGACY_IO); } -static unsigned int -intel_vga_set_decode(struct pci_dev *pdev, bool enable_decode) -{ - struct drm_i915_private *i915 = pdev_to_i915(pdev); - - intel_gmch_vga_set_state(i915, enable_decode); - - if (enable_decode) - return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; - else - return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; -} - int intel_vga_register(struct drm_i915_private *i915) { @@ -127,7 +111,7 @@ int intel_vga_register(struct drm_i915_private *i915) * then we do not take part in VGA arbitration and the * vga_client_register() fails with -ENODEV. */ - ret = vga_client_register(pdev, intel_vga_set_decode); + ret = vga_client_register(pdev, intel_gmch_vga_set_decode); if (ret && ret != -ENODEV) return ret; diff --git a/drivers/gpu/drm/i915/soc/intel_gmch.c b/drivers/gpu/drm/i915/soc/intel_gmch.c index 49c7fb16e934..f32e9f78770a 100644 --- a/drivers/gpu/drm/i915/soc/intel_gmch.c +++ b/drivers/gpu/drm/i915/soc/intel_gmch.c @@ -5,6 +5,7 @@ #include <linux/pci.h> #include <linux/pnp.h> +#include <linux/vgaarb.h> #include <drm/drm_managed.h> #include <drm/i915_drm.h> @@ -167,3 +168,16 @@ int intel_gmch_vga_set_state(struct drm_i915_private *i915, bool enable_decode) return 0; } + +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev, bool enable_decode) +{ + struct drm_i915_private *i915 = pdev_to_i915(pdev); + + intel_gmch_vga_set_state(i915, enable_decode); + + if (enable_decode) + return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM | + VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; + else + return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM; +} diff --git a/drivers/gpu/drm/i915/soc/intel_gmch.h b/drivers/gpu/drm/i915/soc/intel_gmch.h index d0133eedc720..23be2d113afd 100644 --- a/drivers/gpu/drm/i915/soc/intel_gmch.h +++ b/drivers/gpu/drm/i915/soc/intel_gmch.h @@ -8,11 +8,13 @@ #include <linux/types.h> +struct pci_dev; struct drm_i915_private; int intel_gmch_bridge_setup(struct drm_i915_private *i915); void intel_gmch_bar_setup(struct drm_i915_private *i915); void intel_gmch_bar_teardown(struct drm_i915_private *i915); int intel_gmch_vga_set_state(struct drm_i915_private *i915, bool enable_decode); +unsigned int intel_gmch_vga_set_decode(struct pci_dev *pdev, bool enable_decode); #endif /* __INTEL_GMCH_H__ */
Some of the VGA functionality is not needed by the proposed Intel Xe driver, while this will be utilized by i915. Created a version of the function to be used exclusively by i915. Xe will implement it's own respective version. v2: Addressed Jani Nikula's review comments. v3: Dropped a duplicate function (Jani) Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/display/intel_vga.c | 18 +----------------- drivers/gpu/drm/i915/soc/intel_gmch.c | 14 ++++++++++++++ drivers/gpu/drm/i915/soc/intel_gmch.h | 2 ++ 3 files changed, 17 insertions(+), 17 deletions(-)