Re: [PATCH v4] drm/i915: Move LRC register offsets to a header file

Chris Wilson
2018-01-23T20:57:58Z

Quoting Michel Thierry (2018-01-23 19:50:57)
> Newer platforms may have subtle offset changes, which will increase the
> number of defines, so it is probably better to start moving them to its
> own header file. Also move the macros used while setting the reg state.
>
> v2: Rename to intel_lrc_reg.h, to be consistent with i915_reg.h and
> intel_guc_reg.h (Chris)
>
> v3: License notice shenanigans.
>
> v4: Documentation/process/coding-style.rst is always right (Chris)
>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> +#define CTX_REG(reg_state, pos, reg, val) do { \
> + (reg_state)[(pos)+0] = i915_mmio_reg_offset(reg); \
> + (reg_state)[(pos)+1] = (val); \
> +} while (0)
> +
> +#define ASSIGN_CTX_PDP(ppgtt, reg_state, n) do { \
> + const u64 _addr = i915_page_dir_dma_addr((ppgtt), (n)); \
> + reg_state[CTX_PDP ## n ## _UDW+1] = upper_32_bits(_addr); \
> + reg_state[CTX_PDP ## n ## _LDW+1] = lower_32_bits(_addr); \

I know this is a mechanical copy, but we should (reg_state)
nevertheless. We might as well go the extra step and do
u32 regs__ = (reg_state); to silence checkpatch (and pos__).

> +} while (0)
> +
> +#define ASSIGN_CTX_PML4(ppgtt, reg_state) do { \
> + reg_state[CTX_PDP0_UDW + 1] = upper_32_bits(px_dma(&ppgtt->pml4)); \
> + reg_state[CTX_PDP0_LDW + 1] = lower_32_bits(px_dma(&ppgtt->pml4)); \

const u64 addr__ = px_dma() as well.

The jury is out on whether this should be one or two patches. Perfection
would probably be the checkpatch corrections followed by the move (that
way, CI shouldn't complain about the violations).
-Chris