Re: [Intel-gfx] [PATCH] drm/i915/display: abstract all vgaarb access to intel_vga.[ch]

2019-10-02 Thread Jani Nikula 
On Tue, 01 Oct 2019, Ville Syrjälä  wrote:
> On Tue, Oct 01, 2019 at 06:25:06PM +0300, Jani Nikula wrote:
>> Split out the code related to vga client and vgaarb all over the place
>> into new intel_vga.[ch]. No functional changes.
>> 
>> Cc: Ville Syrjälä 
>> Cc: Chris Wilson 
>> Signed-off-by: Jani Nikula 
>> ---
>>  drivers/gpu/drm/i915/Makefile |   3 +-
>>  drivers/gpu/drm/i915/display/intel_display.c  |  97 +--
>>  drivers/gpu/drm/i915/display/intel_display.h  |   3 -
>>  .../drm/i915/display/intel_display_power.c|  24 +--
>>  drivers/gpu/drm/i915/display/intel_vga.c  | 160 ++
>>  drivers/gpu/drm/i915/display/intel_vga.h  |  18 ++
>>  drivers/gpu/drm/i915/i915_drv.c   |  30 +---
>>  drivers/gpu/drm/i915/i915_pci.c   |   1 -
>>  drivers/gpu/drm/i915/i915_suspend.c   |   3 +-
>>  drivers/gpu/drm/i915/intel_runtime_pm.c   |   1 -
>>  10 files changed, 194 insertions(+), 146 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_vga.c
>>  create mode 100644 drivers/gpu/drm/i915/display/intel_vga.h
>> 
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index e04463d85401..d2b53b5add81 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -184,7 +184,8 @@ i915-y += \
>>  display/intel_psr.o \
>>  display/intel_quirks.o \
>>  display/intel_sprite.o \
>> -display/intel_tc.o
>> +display/intel_tc.o \
>> +display/intel_vga.o
>>  i915-$(CONFIG_ACPI) += \
>>  display/intel_acpi.o \
>>  display/intel_opregion.o
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
>> b/drivers/gpu/drm/i915/display/intel_display.c
>> index f1328c08f4ad..d99c59e97568 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -31,7 +31,6 @@
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>>  
>>  #include 
>>  #include 
>> @@ -79,6 +78,7 @@
>>  #include "intel_sideband.h"
>>  #include "intel_sprite.h"
>>  #include "intel_tc.h"
>> +#include "intel_vga.h"
>>  
>>  /* Primary plane formats for gen <= 3 */
>>  static const u32 i8xx_primary_formats[] = {
>> @@ -4241,7 +4241,7 @@ __intel_display_resume(struct drm_device *dev,
>>  int i, ret;
>>  
>>  intel_modeset_setup_hw_state(dev, ctx);
>> -i915_redisable_vga(to_i915(dev));
>> +intel_vga_redisable(to_i915(dev));
>>  
>>  if (!state)
>>  return 0;
>> @@ -15994,35 +15994,6 @@ void intel_init_display_hooks(struct 
>> drm_i915_private *dev_priv)
>>  
>>  }
>>  
>> -static i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv)
>> -{
>> -if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>> -return VLV_VGACNTRL;
>> -else if (INTEL_GEN(dev_priv) >= 5)
>> -return CPU_VGACNTRL;
>> -else
>> -return VGACNTRL;
>> -}
>> -
>> -/* Disable the VGA plane that we never use */
>> -static void i915_disable_vga(struct drm_i915_private *dev_priv)
>> -{
>> -struct pci_dev *pdev = dev_priv->drm.pdev;
>> -u8 sr1;
>> -i915_reg_t vga_reg = i915_vgacntrl_reg(dev_priv);
>> -
>> -/* WaEnableVGAAccessThroughIOPort:ctg,elk,ilk,snb,ivb,vlv,hsw */
>> -vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
>> -outb(SR01, VGA_SR_INDEX);
>> -sr1 = inb(VGA_SR_DATA);
>> -outb(sr1 | 1<<5, VGA_SR_DATA);
>> -vga_put(pdev, VGA_RSRC_LEGACY_IO);
>> -udelay(300);
>> -
>> -I915_WRITE(vga_reg, VGA_DISP_DISABLE);
>> -POSTING_READ(vga_reg);
>> -}
>> -
>>  void intel_modeset_init_hw(struct drm_i915_private *i915)
>>  {
>>  intel_update_cdclk(i915);
>> @@ -16288,7 +16259,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
>>  intel_update_max_cdclk(i915);
>>  
>>  /* Just disable it once at startup */
>> -i915_disable_vga(i915);
>> +intel_vga_disable(i915);
>>  intel_setup_outputs(i915);
>>  
>>  drm_modeset_lock_all(dev);
>> @@ -16647,39 +16618,6 @@ static void intel_sanitize_encoder(struct 
>> intel_encoder *encoder)
>>  icl_sanitize_encoder_pll_mapping(encoder);
>>  }
>>  
>> -void i915_redisable_vga_power_on(struct drm_i915_private *dev_priv)
>> -{
>> -i915_reg_t vga_reg = i915_vgacntrl_reg(dev_priv);
>> -
>> -if (!(I915_READ(vga_reg) & VGA_DISP_DISABLE)) {
>> -DRM_DEBUG_KMS("Something enabled VGA plane, disabling it\n");
>> -i915_disable_vga(dev_priv);
>> -}
>> -}
>> -
>> -void i915_redisable_vga(struct drm_i915_private *dev_priv)
>> -{
>> -intel_wakeref_t wakeref;
>> -
>> -/*
>> - * This function can be called both from intel_modeset_setup_hw_state or
>> - * at a very early point in our resume sequence, where the power well
>> - * structures are not yet restored. Since this function is at a very
>> - * paranoid "someone might have enabled VGA while we were not looking"
>> - * level, just check if the power

Re: [Intel-gfx] [PATCH] drm/i915/display: abstract all vgaarb access to intel_vga.[ch]

2019-10-01 Thread Ville Syrjälä 
On Tue, Oct 01, 2019 at 06:25:06PM +0300, Jani Nikula wrote:
> Split out the code related to vga client and vgaarb all over the place
> into new intel_vga.[ch]. No functional changes.
> 
> Cc: Ville Syrjälä 
> Cc: Chris Wilson 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/Makefile |   3 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |  97 +--
>  drivers/gpu/drm/i915/display/intel_display.h  |   3 -
>  .../drm/i915/display/intel_display_power.c|  24 +--
>  drivers/gpu/drm/i915/display/intel_vga.c  | 160 ++
>  drivers/gpu/drm/i915/display/intel_vga.h  |  18 ++
>  drivers/gpu/drm/i915/i915_drv.c   |  30 +---
>  drivers/gpu/drm/i915/i915_pci.c   |   1 -
>  drivers/gpu/drm/i915/i915_suspend.c   |   3 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c   |   1 -
>  10 files changed, 194 insertions(+), 146 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_vga.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_vga.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e04463d85401..d2b53b5add81 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -184,7 +184,8 @@ i915-y += \
>   display/intel_psr.o \
>   display/intel_quirks.o \
>   display/intel_sprite.o \
> - display/intel_tc.o
> + display/intel_tc.o \
> + display/intel_vga.o
>  i915-$(CONFIG_ACPI) += \
>   display/intel_acpi.o \
>   display/intel_opregion.o
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index f1328c08f4ad..d99c59e97568 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -31,7 +31,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  
>  #include 
>  #include 
> @@ -79,6 +78,7 @@
>  #include "intel_sideband.h"
>  #include "intel_sprite.h"
>  #include "intel_tc.h"
> +#include "intel_vga.h"
>  
>  /* Primary plane formats for gen <= 3 */
>  static const u32 i8xx_primary_formats[] = {
> @@ -4241,7 +4241,7 @@ __intel_display_resume(struct drm_device *dev,
>   int i, ret;
>  
>   intel_modeset_setup_hw_state(dev, ctx);
> - i915_redisable_vga(to_i915(dev));
> + intel_vga_redisable(to_i915(dev));
>  
>   if (!state)
>   return 0;
> @@ -15994,35 +15994,6 @@ void intel_init_display_hooks(struct 
> drm_i915_private *dev_priv)
>  
>  }
>  
> -static i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv)
> -{
> - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> - return VLV_VGACNTRL;
> - else if (INTEL_GEN(dev_priv) >= 5)
> - return CPU_VGACNTRL;
> - else
> - return VGACNTRL;
> -}
> -
> -/* Disable the VGA plane that we never use */
> -static void i915_disable_vga(struct drm_i915_private *dev_priv)
> -{
> - struct pci_dev *pdev = dev_priv->drm.pdev;
> - u8 sr1;
> - i915_reg_t vga_reg = i915_vgacntrl_reg(dev_priv);
> -
> - /* WaEnableVGAAccessThroughIOPort:ctg,elk,ilk,snb,ivb,vlv,hsw */
> - vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> - outb(SR01, VGA_SR_INDEX);
> - sr1 = inb(VGA_SR_DATA);
> - outb(sr1 | 1<<5, VGA_SR_DATA);
> - vga_put(pdev, VGA_RSRC_LEGACY_IO);
> - udelay(300);
> -
> - I915_WRITE(vga_reg, VGA_DISP_DISABLE);
> - POSTING_READ(vga_reg);
> -}
> -
>  void intel_modeset_init_hw(struct drm_i915_private *i915)
>  {
>   intel_update_cdclk(i915);
> @@ -16288,7 +16259,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
>   intel_update_max_cdclk(i915);
>  
>   /* Just disable it once at startup */
> - i915_disable_vga(i915);
> + intel_vga_disable(i915);
>   intel_setup_outputs(i915);
>  
>   drm_modeset_lock_all(dev);
> @@ -16647,39 +16618,6 @@ static void intel_sanitize_encoder(struct 
> intel_encoder *encoder)
>   icl_sanitize_encoder_pll_mapping(encoder);
>  }
>  
> -void i915_redisable_vga_power_on(struct drm_i915_private *dev_priv)
> -{
> - i915_reg_t vga_reg = i915_vgacntrl_reg(dev_priv);
> -
> - if (!(I915_READ(vga_reg) & VGA_DISP_DISABLE)) {
> - DRM_DEBUG_KMS("Something enabled VGA plane, disabling it\n");
> - i915_disable_vga(dev_priv);
> - }
> -}
> -
> -void i915_redisable_vga(struct drm_i915_private *dev_priv)
> -{
> - intel_wakeref_t wakeref;
> -
> - /*
> -  * This function can be called both from intel_modeset_setup_hw_state or
> -  * at a very early point in our resume sequence, where the power well
> -  * structures are not yet restored. Since this function is at a very
> -  * paranoid "someone might have enabled VGA while we were not looking"
> -  * level, just check if the power well is enabled instead of trying to
> -  * follow the "don't touch the power well if we don't need it" policy
> -  * th

[Intel-gfx] [PATCH] drm/i915/display: abstract all vgaarb access to intel_vga.[ch]

2019-10-01 Thread Jani Nikula 
Split out the code related to vga client and vgaarb all over the place
into new intel_vga.[ch]. No functional changes.

Cc: Ville Syrjälä 
Cc: Chris Wilson 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/Makefile |   3 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  97 +--
 drivers/gpu/drm/i915/display/intel_display.h  |   3 -
 .../drm/i915/display/intel_display_power.c|  24 +--
 drivers/gpu/drm/i915/display/intel_vga.c  | 160 ++
 drivers/gpu/drm/i915/display/intel_vga.h  |  18 ++
 drivers/gpu/drm/i915/i915_drv.c   |  30 +---
 drivers/gpu/drm/i915/i915_pci.c   |   1 -
 drivers/gpu/drm/i915/i915_suspend.c   |   3 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c   |   1 -
 10 files changed, 194 insertions(+), 146 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_vga.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_vga.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e04463d85401..d2b53b5add81 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -184,7 +184,8 @@ i915-y += \
display/intel_psr.o \
display/intel_quirks.o \
display/intel_sprite.o \
-   display/intel_tc.o
+   display/intel_tc.o \
+   display/intel_vga.o
 i915-$(CONFIG_ACPI) += \
display/intel_acpi.o \
display/intel_opregion.o
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index f1328c08f4ad..d99c59e97568 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -31,7 +31,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -79,6 +78,7 @@
 #include "intel_sideband.h"
 #include "intel_sprite.h"
 #include "intel_tc.h"
+#include "intel_vga.h"
 
 /* Primary plane formats for gen <= 3 */
 static const u32 i8xx_primary_formats[] = {
@@ -4241,7 +4241,7 @@ __intel_display_resume(struct drm_device *dev,
int i, ret;
 
intel_modeset_setup_hw_state(dev, ctx);
-   i915_redisable_vga(to_i915(dev));
+   intel_vga_redisable(to_i915(dev));
 
if (!state)
return 0;
@@ -15994,35 +15994,6 @@ void intel_init_display_hooks(struct drm_i915_private 
*dev_priv)
 
 }
 
-static i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv)
-{
-   if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-   return VLV_VGACNTRL;
-   else if (INTEL_GEN(dev_priv) >= 5)
-   return CPU_VGACNTRL;
-   else
-   return VGACNTRL;
-}
-
-/* Disable the VGA plane that we never use */
-static void i915_disable_vga(struct drm_i915_private *dev_priv)
-{
-   struct pci_dev *pdev = dev_priv->drm.pdev;
-   u8 sr1;
-   i915_reg_t vga_reg = i915_vgacntrl_reg(dev_priv);
-
-   /* WaEnableVGAAccessThroughIOPort:ctg,elk,ilk,snb,ivb,vlv,hsw */
-   vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
-   outb(SR01, VGA_SR_INDEX);
-   sr1 = inb(VGA_SR_DATA);
-   outb(sr1 | 1<<5, VGA_SR_DATA);
-   vga_put(pdev, VGA_RSRC_LEGACY_IO);
-   udelay(300);
-
-   I915_WRITE(vga_reg, VGA_DISP_DISABLE);
-   POSTING_READ(vga_reg);
-}
-
 void intel_modeset_init_hw(struct drm_i915_private *i915)
 {
intel_update_cdclk(i915);
@@ -16288,7 +16259,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
intel_update_max_cdclk(i915);
 
/* Just disable it once at startup */
-   i915_disable_vga(i915);
+   intel_vga_disable(i915);
intel_setup_outputs(i915);
 
drm_modeset_lock_all(dev);
@@ -16647,39 +16618,6 @@ static void intel_sanitize_encoder(struct 
intel_encoder *encoder)
icl_sanitize_encoder_pll_mapping(encoder);
 }
 
-void i915_redisable_vga_power_on(struct drm_i915_private *dev_priv)
-{
-   i915_reg_t vga_reg = i915_vgacntrl_reg(dev_priv);
-
-   if (!(I915_READ(vga_reg) & VGA_DISP_DISABLE)) {
-   DRM_DEBUG_KMS("Something enabled VGA plane, disabling it\n");
-   i915_disable_vga(dev_priv);
-   }
-}
-
-void i915_redisable_vga(struct drm_i915_private *dev_priv)
-{
-   intel_wakeref_t wakeref;
-
-   /*
-* This function can be called both from intel_modeset_setup_hw_state or
-* at a very early point in our resume sequence, where the power well
-* structures are not yet restored. Since this function is at a very
-* paranoid "someone might have enabled VGA while we were not looking"
-* level, just check if the power well is enabled instead of trying to
-* follow the "don't touch the power well if we don't need it" policy
-* the rest of the driver uses.
-*/
-   wakeref = intel_display_power_get_if_enabled(dev_priv,
-POWER_DOMAIN_VGA);
-   if (!wakeref)
-   return;
-
-