On Thu, 21 Mar 2019, José Roberto de Souza <jose.souza@xxxxxxxxx> wrote:
> Right now it have a mix of PSR registers that are relative to PSR
> mmio base and other register with a hardcoded address, lets keep it
> consistented and have it all relative to mmio base.
This is not strictly limited to this patch, but an overall trend. The
thing that really bugs me with this is losing more of the actual
absolute mmio addresses from the file. When you're seeking to add a new
register, you can't trivially grep for it in the file anymore. Not all
of our register names match the spec (and the spec occasionally also
changes register names) so being able to find the offset is important.
When we added the macros that use ->pipe_offsets and ->trans_offsets, we
took care to have at least one of the offsets in the file. I'm wondering
if we could do something like that here as well.
BR,
Jani.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 28728399e607..e1ed2ba1c315 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4326,7 +4326,7 @@ enum {
> #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */
> #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
>
> -#define EDP_PSR2_CTL _MMIO(0x6f900)
> +#define EDP_PSR2_CTL _MMIO(dev_priv->psr.mmio_base + 0x100)
> #define EDP_PSR2_ENABLE (1 << 31)
> #define EDP_SU_TRACK_ENABLE (1 << 30)
> #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */
> @@ -4344,7 +4344,7 @@ enum {
> #define EDP_PSR2_IDLE_FRAME_MASK 0xf
> #define EDP_PSR2_IDLE_FRAME_SHIFT 0
>
> -#define PSR_EVENT _MMIO(0x6F848)
> +#define PSR_EVENT _MMIO(dev_priv->psr.mmio_base + 0x48)
> #define PSR_EVENT_PSR2_WD_TIMER_EXPIRE (1 << 17)
> #define PSR_EVENT_PSR2_DISABLED (1 << 16)
> #define PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN (1 << 15)
> @@ -4362,14 +4362,11 @@ enum {
> #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1)
> #define PSR_EVENT_PSR_DISABLE (1 << 0)
>
> -#define EDP_PSR2_STATUS _MMIO(0x6f940)
> +#define EDP_PSR2_STATUS _MMIO(dev_priv->psr.mmio_base + 0x140)
> #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28)
> #define EDP_PSR2_STATUS_STATE_SHIFT 28
>
> -#define _PSR2_SU_STATUS_0 0x6F914
> -#define _PSR2_SU_STATUS_1 0x6F918
> -#define _PSR2_SU_STATUS_2 0x6F91C
> -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1))
> +#define _PSR2_SU_STATUS(index) _MMIO(dev_priv->psr.mmio_base + 0x114 + (index) * 4)
> #define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame) / 3))
> #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10)
> #define PSR2_SU_STATUS_MASK(frame) (0x3ff << PSR2_SU_STATUS_SHIFT(frame))
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
