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

Jani Nikula
2015-06-10T08:07:35Z

On Tue, 09 Jun 2015, Dave Gordon <david.s.gordon@intel.com> wrote:
> 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.

It's not inconsistent, it's consistent on another level:

We've settled on including the mmio_offset only for macros that need
it. If a macro is relevant only on platforms that all have the same mmio
offset, the offset is included statically (currently 0 or
VLV_DISPLAY_BASE). dev_priv->info.display_mmio_offset is only used when
the macro is relevant on platforms with different mmio offsets. This we
try to follow consistently.

> 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.

Agreed in principle, but I think we've lost the battle. It's way too
much churn, and makes a lot of code really ugly. Compare these:

I915_WRITE(FOOBAR, I915_READ(FOOBAR) | FOOBAR_ENABLE);

I915_WRITE(dev_priv, FOOBAR(dev_priv),
I915_READ(dev_priv, FOOBAR(dev_priv)) | FOOBAR_ENABLE);

We'll try to focus on only requiring "dev_priv" implicitly and nothing
else, and where a parameter does get passed, we'll try to make it
"dev_priv" as that pretty much has to be around anyway.

BR,
Jani.

--
Jani Nikula, Intel Open Source Technology Center