On Wed, Feb 02, 2022 at 11:17:11AM +0200, Jani Nikula wrote: > On Wed, 12 Jan 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > On Tue, Jan 11, 2022 at 10:55:44AM +0200, Jani Nikula wrote: > >> On Mon, 10 Jan 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> > >> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > >> >> --- > >> >> 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. -- Ville Syrjälä Intel
