Re: [PATCH v2 1/1] drm/i915: Save PM interrupt register offsets in device info

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 24 Oct 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Sagar Arun Kamble (2017-10-24 11:41:13)
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index 875d428..d1a4911 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -462,4 +462,15 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>>                          info->sseu.has_subslice_pg ? "y" : "n");
>>         DRM_DEBUG_DRIVER("has EU power gating: %s\n",
>>                          info->sseu.has_eu_pg ? "y" : "n");
>> +
>> +       /* Initialize PM interrupt register offsets */
>> +       if (INTEL_GEN(dev_priv) >= 8) {
>> +               info->pm_iir_offset = GEN8_GT_IIR(2);
>> +               info->pm_imr_offset = GEN8_GT_IMR(2);
>> +               info->pm_ier_offset = GEN8_GT_IER(2);
>> +       } else {
>> +               info->pm_iir_offset = GEN6_PMIIR;
>> +               info->pm_imr_offset = GEN6_PMIMR;
>> +               info->pm_ier_offset = GEN6_PMIER;
>> +       }
>
> If you are going to take another pass at this, move these into the
> static tables in i915_pci.c
>
> Updating GEN6_FEATURES and GEN8_FEATURES will then percolate into
> individual platform defines.

Like I wrote in reply to v1, I'm not convinced we should do this at all.

What makes *these* registers so important they must be in device info?
What makes most of i915_reg.h so unimportant they don't deserve the same
treatment? Where do you draw the line?

I'd draw the line at, no registers at device info.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx