Re: [RFC 1/7] drm/i915/dsi: Define command mode registers

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

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> Adding all the register definitions needed
> for mipi dsi command mode.
>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 76 +++++++++++++++++++++++++++++----
>  1 file changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e24991e54897..73bc85855b79 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4919,14 +4919,20 @@ enum {
>  #define   BLM_PCH_POLARITY			(1 << 29)
>  #define BLC_PWM_PCH_CTL2	_MMIO(0xc8254)
>  
> -#define UTIL_PIN_CTL		_MMIO(0x48400)
> -#define   UTIL_PIN_ENABLE	(1 << 31)
> -
> -#define   UTIL_PIN_PIPE(x)     ((x) << 29)
> -#define   UTIL_PIN_PIPE_MASK   (3 << 29)
> -#define   UTIL_PIN_MODE_PWM    (1 << 24)
> -#define   UTIL_PIN_MODE_MASK   (0xf << 24)
> -#define   UTIL_PIN_POLARITY    (1 << 22)
> +#define UTIL_PIN_CTL			_MMIO(0x48400)
> +#define  UTIL_PIN_ENABLE		(1 << 31)
> +#define  UTIL_PIN_PIPE_MASK		(3 << 29)
> +#define  UTIL_PIN_PIPE(x)		((x) << 29)
> +#define  UTIL_PIN_MODE_MASK		(0xf << 24)
> +#define  UTIL_PIN_MODE_DATA		(0 << 24)
> +#define  UTIL_PIN_MODE_PWM		(1 << 24)
> +#define  UTIL_PIN_MODE_VBLANK		(4 << 24)
> +#define  UTIL_PIN_MODE_VSYNC		(5 << 24)
> +#define  UTIL_PIN_MODE_EYE_LEVEL	(8 << 24)
> +#define  UTIL_PIN_OP_DATA		(1 << 23)
> +#define  UTIL_PIN_POLARITY		(1 << 22)
> +#define  ICL_UTIL_PIN_DIRECTION		(1 << 19)
> +#define  ICL_UTIL_PIN_IP_DATA		(1 << 16)
          ^^

If you change/add everything, please put three spaces there.

Agreed on dropping the ICL prefix here, mainly because the added bits
predate ICL.

On naming, OP and IP aren't from the spec and aren't
self-explanatory. I'd go with the full UTIL_PIN_OUTPUT_DATA and
UTIL_PIN_INPUT_DATA. I think the direction bit should say what the
direction is when it's set, so you don't have to look it up.




>  
>  /* BXT backlight register definition. */
>  #define _BXT_BLC_PWM_CTL1			0xC8250
> @@ -7407,6 +7413,8 @@ enum {
>  #define GEN8_DE_PORT_IMR _MMIO(0x44444)
>  #define GEN8_DE_PORT_IIR _MMIO(0x44448)
>  #define GEN8_DE_PORT_IER _MMIO(0x4444c)
> +#define  ICL_DSI_1			(1 << 31)
> +#define  ICL_DSI_0			(1 << 30)
>  #define  ICL_AUX_CHANNEL_E		(1 << 29)
>  #define  CNL_AUX_CHANNEL_F		(1 << 28)
>  #define  GEN9_AUX_CHANNEL_D		(1 << 27)
> @@ -10659,6 +10667,57 @@ enum skl_power_gate {
>  #define  ICL_ESC_CLK_DIV_SHIFT			0
>  #define DSI_MAX_ESC_CLK			20000		/* in KHz */
>  
> +#define _ICL_DSI_CMD_FRMCTL_0		0x6b034
> +#define _ICL_DSI_CMD_FRMCTL_1		0x6b834
> +#define ICL_DSI_CMD_FRMCTL(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_CMD_FRMCTL_0,\
> +						  _ICL_DSI_CMD_FRMCTL_1)
> +#define  ICL_FRAME_UPDATE_REQUEST		(1 << 31)
> +#define  ICL_PERIODIC_FRAME_UPDATE_ENABLE	(1 << 29)
> +#define  ICL_NULL_PACKET_ENABLE			(1 << 28)
> +#define  ICL_FRAME_IN_PROGRESS			(1 << 0)
> +
> +#define _ICL_DSI_INTR_MASK_REG_0		0x6b070
> +#define _ICL_DSI_INTR_MASK_REG_1		0x6b870
> +#define ICL_DSI_INTR_MASK_REG(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_INTR_MASK_REG_0,\
> +						  _ICL_DSI_INTR_MASK_REG_1)
> +
> +#define _ICL_DSI_INTR_IDENT_REG_0		0x6b074
> +#define _ICL_DSI_INTR_IDENT_REG_1		0x6b874
> +#define ICL_DSI_INTR_IDENT_REG(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_INTR_IDENT_REG_0,\
> +						  _ICL_DSI_INTR_IDENT_REG_1)
> +#define  ICL_TE_EVENT				(1 << 31)
> +#define  ICL_RX_DATA_OR_BTA_TERMINATED		(1 << 30)
> +#define  ICL_TX_DATA				(1 << 29)
> +#define  ICL_ULPS_ENTRY_DONE			(1 << 28)
> +#define  ICL_NON_TE_TRIGGER_RECEIVED		(1 << 27)
> +#define  ICL_HOST_CHKSUM_ERROR			(1 << 26)
> +#define  ICL_HOST_MULTI_ECC_ERROR		(1 << 25)
> +#define  ICL_HOST_SINGL_ECC_ERROR		(1 << 24)
> +#define  ICL_HOST_CONTENTION_DETECTED		(1 << 23)
> +#define  ICL_HOST_FALSE_CONTROL_ERROR		(1 << 22)
> +#define  ICL_HOST_TIMEOUT_ERROR			(1 << 21)
> +#define  ICL_HOST_LOW_POWER_TX_SYNC_ERROR	(1 << 20)
> +#define  ICL_HOST_ESCAPE_MODE_ENTRY_ERROR	(1 << 19)
> +#define  ICL_FRAME_UPDATE_DONE			(1 << 16)
> +#define  ICL_PROTOCOL_VIOLATION_REPORTED	(1 << 15)
> +#define  ICL_INVALID_TX_LENGTH			(1 << 13)
> +#define  ICL_INVALID_VC				(1 << 12)
> +#define  ICL_INVALID_DATA_TYPE			(1 << 11)
> +#define  ICL_PERIPHERAL_CHKSUM_ERROR		(1 << 10)
> +#define  ICL_PERIPHERAL_MULTI_ECC_ERROR		(1 << 9)
> +#define  ICL_PERIPHERAL_SINGLE_ECC_ERROR	(1 << 8)
> +#define  ICL_PERIPHERAL_CONTENTION_DETECTED	(1 << 7)
> +#define  ICL_PERIPHERAL_FALSE_CTRL_ERROR	(1 << 6)
> +#define  ICL_PERIPHERAL_TIMEOUT_ERROR		(1 << 5)
> +#define  ICL_PERIPHERAL_LP_TX_SYNC_ERROR	(1 << 4)
> +#define  ICL_PERIPHERAL_ESC_MODE_ENTRY_CMD_ERROR	(1 << 3)
> +#define  ICL_EOT_SYNC_ERROR			(1 << 2)
> +#define  ICL_SOT_SYNC_ERROR			(1 << 1)
> +#define  ICL_SOT_ERROR				(1 << 0)

All the ICL DSI register defs start with DSI_ which distinguishes them
from VLV DSI which has the MIPI_ prefix. It's not perfect, but I think
clean enough that we don't need the ICL_ prefix here.

BR,
Jani.

> +
>  /* Gen4+ Timestamp and Pipe Frame time stamp registers */
>  #define GEN4_TIMESTAMP		_MMIO(0x2358)
>  #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
> @@ -11263,6 +11322,7 @@ enum skl_power_gate {
>  #define  CMD_MODE_TE_GATE		(0x1 << 28)
>  #define  VIDEO_MODE_SYNC_EVENT		(0x2 << 28)
>  #define  VIDEO_MODE_SYNC_PULSE		(0x3 << 28)
> +#define  TE_SOURCE_GPIO			(1 << 27)
>  #define  LINK_READY			(1 << 20)
>  #define  PIX_FMT_MASK			(0x3 << 16)
>  #define  PIX_FMT_SHIFT			16

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