Re: [PATCH] drm/i915: Correcting the reg definitions for PORT_DFT

Dave Gordon
2015-06-09T15:19:41Z

On 05/06/15 14:08, Ville Syrjälä wrote:
> On Fri, Jun 05, 2015 at 02:00:24PM +0100, Minu Mathai wrote:
>> From: Minu <minu.mathai@intel.com>
>>
>> Display CRCs were not readable because the register defintions
>> for PORT_DFT_I9XX and PORT_DFT2_G4X were wrong.
>> MMIO offset needs to be added to these register offsets to fix them.
>>
>> Issue: GMINL-6869
>> Signed-off-by: Minu Mathai <minu.mathai@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 7213224..c327c7c 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3193,7 +3193,7 @@ enum skl_disp_power_wells {
>> #define PCH_HDMIC 0xe1150
>> #define PCH_HDMID 0xe1160
>>
>> -#define PORT_DFT_I9XX 0x61150
>> +#define PORT_DFT_I9XX (dev_priv->info.display_mmio_offset + 0x61150)
>
> PORT_DFT_I9XX isn't used on VLV/CHV, so this doesn't change anything.
>
>> #define DC_BALANCE_RESET (1 << 25)
>> #define PORT_DFT2_G4X (dev_priv->info.display_mmio_offset + 0x61154)
>> #define DC_BALANCE_RESET_VLV (1 << 31)

Regardless of whether it's used, we have an inconsistency between the
definitions of PORT_DFT_I9XX and PORT_DFT2_G4X -- one includes the
mmio_offset and the other doesn't. Personally I think the #define with
an implicit dependency on an object called "dev_priv" is really ugly and
we should move away from that style rather than adding mode of them, but
that's a lot of work.

As Minu says PORT_DFT_I9XX isn't really needed after all, can we just
delete it to remove the inconsistency?

.Dave.