diff mbox series

[3/4] drm/i915/vga: switch to use VGA definitions from video/vga.h

Message ID 20220110095740.166078-3-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/4] drm/i915: split out PCI config space registers from i915_reg.h | expand

Commit Message

Jani Nikula Jan. 10, 2022, 9:57 a.m. UTC
The video/vga.h has macros for the VGA registers. Switch to use them.

Suggested-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Ville Syrjälä Jan. 10, 2022, 4:03 p.m. UTC | #1
On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
> The video/vga.h has macros for the VGA registers. Switch to use them.
> 
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
> index fa779f7ea415..43c12036c1fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_vga.c
> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
> @@ -7,6 +7,7 @@
>  #include <linux/vgaarb.h>
>  
>  #include <drm/i915_drm.h>
> +#include <video/vga.h>
>  
>  #include "i915_drv.h"
>  #include "intel_de.h"
> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);

Not a huge fan of some of these defines since now I have
no idea what register this is selecting.

> +	sr1 = inb(VGA_SEQ_D);
> +	outb(sr1 | VGA_SR01_SCREEN_OFF, VGA_SEQ_D);
>  	vga_put(pdev, VGA_RSRC_LEGACY_IO);
>  	udelay(300);
>  
> @@ -92,7 +93,7 @@ void intel_vga_reset_io_mem(struct drm_i915_private *i915)
>  	 * and error messages.
>  	 */
>  	vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> -	outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
> +	outb(inb(VGA_MIS_R), VGA_MIS_W);
>  	vga_put(pdev, VGA_RSRC_LEGACY_IO);
>  }
>  
> -- 
> 2.30.2
Jani Nikula Jan. 11, 2022, 8:55 a.m. UTC | #2
On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
>> The video/vga.h has macros for the VGA registers. Switch to use them.
>> 
>> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
>> index fa779f7ea415..43c12036c1fa 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vga.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
>> @@ -7,6 +7,7 @@
>>  #include <linux/vgaarb.h>
>>  
>>  #include <drm/i915_drm.h>
>> +#include <video/vga.h>
>>  
>>  #include "i915_drv.h"
>>  #include "intel_de.h"
>> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
>> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
>
> Not a huge fan of some of these defines since now I have
> no idea what register this is selecting.

It's a bit silly that we have our own macros for this stuff, but I get
the point. Took me a while to figure the changes out because the macros
in video/vga.h aren't even grouped in a helpful way.

I guess you'd prefer patch [1] over patches 3-4 in this series then? For
me the main goal is to just reduce the size of i915_reg.h.

BR,
Jani.


[1] https://patchwork.freedesktop.org/patch/msgid/20220107094951.96181-2-jani.nikula@intel.com


>
>> +	sr1 = inb(VGA_SEQ_D);
>> +	outb(sr1 | VGA_SR01_SCREEN_OFF, VGA_SEQ_D);
>>  	vga_put(pdev, VGA_RSRC_LEGACY_IO);
>>  	udelay(300);
>>  
>> @@ -92,7 +93,7 @@ void intel_vga_reset_io_mem(struct drm_i915_private *i915)
>>  	 * and error messages.
>>  	 */
>>  	vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
>> -	outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
>> +	outb(inb(VGA_MIS_R), VGA_MIS_W);
>>  	vga_put(pdev, VGA_RSRC_LEGACY_IO);
>>  }
>>  
>> -- 
>> 2.30.2
Lucas De Marchi Jan. 11, 2022, 4:14 p.m. UTC | #3
On Tue, Jan 11, 2022 at 10:55:44AM +0200, Jani Nikula wrote:
>On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
>>> The video/vga.h has macros for the VGA registers. Switch to use them.
>>>
>>> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
>>> index fa779f7ea415..43c12036c1fa 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_vga.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
>>> @@ -7,6 +7,7 @@
>>>  #include <linux/vgaarb.h>
>>>
>>>  #include <drm/i915_drm.h>
>>> +#include <video/vga.h>
>>>
>>>  #include "i915_drv.h"
>>>  #include "intel_de.h"
>>> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
>>> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
>>
>> Not a huge fan of some of these defines since now I have
>> no idea what register this is selecting.
>
>It's a bit silly that we have our own macros for this stuff, but I get
>the point. Took me a while to figure the changes out because the macros
>in video/vga.h aren't even grouped in a helpful way.
>
>I guess you'd prefer patch [1] over patches 3-4 in this series then? For
>me the main goal is to just reduce the size of i915_reg.h.

alternatively, to patch video/vga.h to make it pretty?

Lucas De Marchi
Jani Nikula Jan. 11, 2022, 4:19 p.m. UTC | #4
On Tue, 11 Jan 2022, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Tue, Jan 11, 2022 at 10:55:44AM +0200, Jani Nikula wrote:
>>On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>> On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
>>>> The video/vga.h has macros for the VGA registers. Switch to use them.
>>>>
>>>> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
>>>> index fa779f7ea415..43c12036c1fa 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_vga.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
>>>> @@ -7,6 +7,7 @@
>>>>  #include <linux/vgaarb.h>
>>>>
>>>>  #include <drm/i915_drm.h>
>>>> +#include <video/vga.h>
>>>>
>>>>  #include "i915_drv.h"
>>>>  #include "intel_de.h"
>>>> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
>>>> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
>>>
>>> Not a huge fan of some of these defines since now I have
>>> no idea what register this is selecting.
>>
>>It's a bit silly that we have our own macros for this stuff, but I get
>>the point. Took me a while to figure the changes out because the macros
>>in video/vga.h aren't even grouped in a helpful way.
>>
>>I guess you'd prefer patch [1] over patches 3-4 in this series then? For
>>me the main goal is to just reduce the size of i915_reg.h.
>
> alternatively, to patch video/vga.h to make it pretty?

If it's enough to just rearrange the stuff, maybe. But if it means
renames, I'm not going to touch a big pile of ancient fb/vga drivers to
chase this one.


BR,
Jani.
Lucas De Marchi Jan. 11, 2022, 4:37 p.m. UTC | #5
On Tue, Jan 11, 2022 at 06:19:10PM +0200, Jani Nikula wrote:
>On Tue, 11 Jan 2022, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> On Tue, Jan 11, 2022 at 10:55:44AM +0200, Jani Nikula wrote:
>>>On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>>> On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
>>>>> The video/vga.h has macros for the VGA registers. Switch to use them.
>>>>>
>>>>> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
>>>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
>>>>> index fa779f7ea415..43c12036c1fa 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_vga.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
>>>>> @@ -7,6 +7,7 @@
>>>>>  #include <linux/vgaarb.h>
>>>>>
>>>>>  #include <drm/i915_drm.h>
>>>>> +#include <video/vga.h>
>>>>>
>>>>>  #include "i915_drv.h"
>>>>>  #include "intel_de.h"
>>>>> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
>>>>> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
>>>>
>>>> Not a huge fan of some of these defines since now I have
>>>> no idea what register this is selecting.
>>>
>>>It's a bit silly that we have our own macros for this stuff, but I get
>>>the point. Took me a while to figure the changes out because the macros
>>>in video/vga.h aren't even grouped in a helpful way.
>>>
>>>I guess you'd prefer patch [1] over patches 3-4 in this series then? For
>>>me the main goal is to just reduce the size of i915_reg.h.
>>
>> alternatively, to patch video/vga.h to make it pretty?
>
>If it's enough to just rearrange the stuff, maybe. But if it means
>renames, I'm not going to touch a big pile of ancient fb/vga drivers to
>chase this one.

I think it would be ok to add them as aliases to the names used in
other places. Then the other places can be converted later if at all.

But not a strong opinion... up to you, Ville and Matt.

Lucas De Marchi
Ville Syrjälä Jan. 12, 2022, 1:24 p.m. UTC | #6
On Tue, Jan 11, 2022 at 10:55:44AM +0200, Jani Nikula wrote:
> On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
> >> The video/vga.h has macros for the VGA registers. Switch to use them.
> >> 
> >> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
> >>  1 file changed, 5 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
> >> index fa779f7ea415..43c12036c1fa 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_vga.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
> >> @@ -7,6 +7,7 @@
> >>  #include <linux/vgaarb.h>
> >>  
> >>  #include <drm/i915_drm.h>
> >> +#include <video/vga.h>
> >>  
> >>  #include "i915_drv.h"
> >>  #include "intel_de.h"
> >> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
> >> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
> >
> > Not a huge fan of some of these defines since now I have
> > no idea what register this is selecting.
> 
> It's a bit silly that we have our own macros for this stuff, but I get
> the point. Took me a while to figure the changes out because the macros
> in video/vga.h aren't even grouped in a helpful way.
> 
> I guess you'd prefer patch [1] over patches 3-4 in this series then? For
> me the main goal is to just reduce the size of i915_reg.h.

I guess another option is to go with this and just
s/VGA_SEQ_CLOCK_MODE/0x01/ or something. I think the rest
of the defines are probably clear enough.
Jani Nikula Feb. 2, 2022, 9:17 a.m. UTC | #7
On Wed, 12 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Jan 11, 2022 at 10:55:44AM +0200, Jani Nikula wrote:
>> On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
>> >> The video/vga.h has macros for the VGA registers. Switch to use them.
>> >> 
>> >> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
>> >>  1 file changed, 5 insertions(+), 4 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
>> >> index fa779f7ea415..43c12036c1fa 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_vga.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
>> >> @@ -7,6 +7,7 @@
>> >>  #include <linux/vgaarb.h>
>> >>  
>> >>  #include <drm/i915_drm.h>
>> >> +#include <video/vga.h>
>> >>  
>> >>  #include "i915_drv.h"
>> >>  #include "intel_de.h"
>> >> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
>> >> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
>> >
>> > Not a huge fan of some of these defines since now I have
>> > no idea what register this is selecting.
>> 
>> It's a bit silly that we have our own macros for this stuff, but I get
>> the point. Took me a while to figure the changes out because the macros
>> in video/vga.h aren't even grouped in a helpful way.
>> 
>> I guess you'd prefer patch [1] over patches 3-4 in this series then? For
>> me the main goal is to just reduce the size of i915_reg.h.
>
> I guess another option is to go with this and just
> s/VGA_SEQ_CLOCK_MODE/0x01/ or something. I think the rest
> of the defines are probably clear enough.

I dropped the ball here a bit. If I respin the same patches, but with
the above line changed to one of these, is it okay? Which do you prefer?

1)	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I); /* SR01 */

2)	#define SR01 VGA_SEQ_CLOCK_MODE
	outb(SR01, VGA_SEQ_I);

3)	outb(0x01, VGA_SEQ_I);


BR,
Jani.
Ville Syrjälä Feb. 2, 2022, 10:22 a.m. UTC | #8
On Wed, Feb 02, 2022 at 11:17:11AM +0200, Jani Nikula wrote:
> On Wed, 12 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Jan 11, 2022 at 10:55:44AM +0200, Jani Nikula wrote:
> >> On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> > On Mon, Jan 10, 2022 at 11:57:39AM +0200, Jani Nikula wrote:
> >> >> The video/vga.h has macros for the VGA registers. Switch to use them.
> >> >> 
> >> >> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> >> ---
> >> >>  drivers/gpu/drm/i915/display/intel_vga.c | 9 +++++----
> >> >>  1 file changed, 5 insertions(+), 4 deletions(-)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
> >> >> index fa779f7ea415..43c12036c1fa 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_vga.c
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
> >> >> @@ -7,6 +7,7 @@
> >> >>  #include <linux/vgaarb.h>
> >> >>  
> >> >>  #include <drm/i915_drm.h>
> >> >> +#include <video/vga.h>
> >> >>  
> >> >>  #include "i915_drv.h"
> >> >>  #include "intel_de.h"
> >> >> @@ -34,9 +35,9 @@ void intel_vga_disable(struct drm_i915_private *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);
> >> >> +	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
> >> >
> >> > Not a huge fan of some of these defines since now I have
> >> > no idea what register this is selecting.
> >> 
> >> It's a bit silly that we have our own macros for this stuff, but I get
> >> the point. Took me a while to figure the changes out because the macros
> >> in video/vga.h aren't even grouped in a helpful way.
> >> 
> >> I guess you'd prefer patch [1] over patches 3-4 in this series then? For
> >> me the main goal is to just reduce the size of i915_reg.h.
> >
> > I guess another option is to go with this and just
> > s/VGA_SEQ_CLOCK_MODE/0x01/ or something. I think the rest
> > of the defines are probably clear enough.
> 
> I dropped the ball here a bit. If I respin the same patches, but with
> the above line changed to one of these, is it okay? Which do you prefer?
> 
> 1)	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I); /* SR01 */
> 
> 2)	#define SR01 VGA_SEQ_CLOCK_MODE
> 	outb(SR01, VGA_SEQ_I);
> 
> 3)	outb(0x01, VGA_SEQ_I);

3 seems like the best option of these.
Jani Nikula Feb. 2, 2022, 11:25 a.m. UTC | #9
On Wed, 02 Feb 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> I dropped the ball here a bit. If I respin the same patches, but with
>> the above line changed to one of these, is it okay? Which do you prefer?
>> 
>> 1)	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I); /* SR01 */
>> 
>> 2)	#define SR01 VGA_SEQ_CLOCK_MODE
>> 	outb(SR01, VGA_SEQ_I);
>> 
>> 3)	outb(0x01, VGA_SEQ_I);
>
> 3 seems like the best option of these.

Thanks, v2 on the list.

BR,
Jani.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
index fa779f7ea415..43c12036c1fa 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -7,6 +7,7 @@ 
 #include <linux/vgaarb.h>
 
 #include <drm/i915_drm.h>
+#include <video/vga.h>
 
 #include "i915_drv.h"
 #include "intel_de.h"
@@ -34,9 +35,9 @@  void intel_vga_disable(struct drm_i915_private *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);
+	outb(VGA_SEQ_CLOCK_MODE, VGA_SEQ_I);
+	sr1 = inb(VGA_SEQ_D);
+	outb(sr1 | VGA_SR01_SCREEN_OFF, VGA_SEQ_D);
 	vga_put(pdev, VGA_RSRC_LEGACY_IO);
 	udelay(300);
 
@@ -92,7 +93,7 @@  void intel_vga_reset_io_mem(struct drm_i915_private *i915)
 	 * and error messages.
 	 */
 	vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
-	outb(inb(VGA_MSR_READ), VGA_MSR_WRITE);
+	outb(inb(VGA_MIS_R), VGA_MIS_W);
 	vga_put(pdev, VGA_RSRC_LEGACY_IO);
 }