Re: [Intel-gfx] [PATCH 3/4] drm/i915/vga: switch to use VGA definitions from video/vga.h

Lucas De Marchi
2022-01-11T16:37:15Z

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