Re: [Intel-gfx] [PATCH v5 12/13] drm/i915/icl: Define TRANS_DSI_FUNC_CONF register
On Wed, 12 Sep 2018, Madhav Chauhan wrote: > On 9/12/2018 1:00 AM, Jani Nikula wrote: >> On Tue, 10 Jul 2018, Madhav Chauhan wrote: >> The convention is to define macros for field values that you can OR >> directly in place instead of requiring a shift. Please stick to the >> conventions. Use _SHIFT and _MASK. >> >> We can debate the relative merits of both approaches at some point, but >> this is not the time. > > Just to understand this point correctly, > > #define OP_MODE(x) ((x) << 28) is OK This is acceptable when the values for the field are *not* defined as separate macros. The convention is that the values for fields are defined already shifted in place, and that would conflict. > but > #define OP_MODE_MASK (0x3 << 28) is NOT OK This is okay and recommended. > and should be: > #define OP_MODE_MASK0x3 This is not okay. > #define OP_MODE_SHIFT 28 This is okay. Please read the big comment with examples near the top of i915_reg.h, and let me know which part is not clear from that. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 12/13] drm/i915/icl: Define TRANS_DSI_FUNC_CONF register
On 9/12/2018 1:00 AM, Jani Nikula wrote:
On Tue, 10 Jul 2018, Madhav Chauhan wrote:
This patch defines transcoder function configuration
registers and its bitfields for both DSI ports.
Used while programming/enabling DSI transcoder.
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/i915_reg.h | 47 +
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 62bc76e..71ce6ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10134,6 +10134,53 @@ enum skl_power_gate {
#define TA_SURE_TIME(x) (x << 16)
#define TA_SURE_TIME_MASK(0x1f << 16)
+/* DSI transcoder configuration */
+#define _DSI_TRANS_FUNC_CONF_0 0x6b030
+#define _DSI_TRANS_FUNC_CONF_1 0x6b830
+#define DSI_TRANS_FUNC_CONF(tc)_MMIO_DSI(tc, \
+ _DSI_TRANS_FUNC_CONF_0,\
+ _DSI_TRANS_FUNC_CONF_1)
+#define OP_MODE(x)(x << 28)
+#define OP_MODE_MASK (0x3 << 28)
+#define CMD_MODE_NO_GATE 0x0
+#define CMD_MODE_TE_GATE 0x1
+#define VIDEO_MODE_SYNC_EVENT 0x2
+#define VIDEO_MODE_SYNC_PULSE 0x3
The convention is to define macros for field values that you can OR
directly in place instead of requiring a shift. Please stick to the
conventions. Use _SHIFT and _MASK.
We can debate the relative merits of both approaches at some point, but
this is not the time.
Just to understand this point correctly,
#define OP_MODE(x) ((x) << 28) is OK
but
#define OP_MODE_MASK (0x3 << 28) is NOT OK
and should be:
#define OP_MODE_MASK0x3
#define OP_MODE_SHIFT 28
Regards,
Madhav
BR,
Jani.
+#define LINK_READY(1 << 20)
+#define PIX_FMT(x)(x << 16)
+#define PIX_FMT_MASK (0x3 << 16)
+#define PIX_FMT_RGB5650x0
+#define PIX_FMT_RGB666_PACKED 0x1
+#define PIX_FMT_RGB666_LOOSE 0x2
+#define PIX_FMT_RGB8880x3
+#define PIX_FMT_RGB101010 0x4
+#define PIX_FMT_RGB121212 0x5
+#define PIX_FMT_COMPRESSED0x6
+#define BGR_TRANSMISSION (1 << 15)
+#define PIX_VIRT_CHAN(x) (x << 12)
+#define PIX_VIRT_CHAN_MASK(0x3 << 12)
+#define PIX_BUF_THRESHOLD(x) ((x & 0x3) << 10)
+#define PIX_BUF_THRESHOLD_MASK(0x3 << 10)
+#define PIX_BUF_THRESHOLD_1_4 0x0
+#define PIX_BUF_THRESHOLD_1_2 0x1
+#define PIX_BUF_THRESHOLD_3_4 0x2
+#define PIX_BUF_THRESHOLD_FULL0x3
+#define CONTINUOUS_CLK(x) (x << 8)
+#define CONTINUOUS_CLK_MASK (0x3 << 8)
+#define CLK_ENTER_LP_AFTER_DATA 0x0
+#define CLK_HS_OR_LP 0x2
+#define CLK_HS_CONTINUOUS 0x3
+#define LINK_CALIBRATION(x) (x << 4)
+#define LINK_CALIBRATION_MASK (0x3 << 4)
+#define CALIBRATION_DISABLED 0x0
+#define CALIBRATION_ENABLED_INITIAL_ONLY 0x2
+#define CALIBRATION_ENABLED_INITIAL_PERIODIC 0x3
+#define S3D_ORIENTATION(x)(x << 1)
+#define S3D_ORIENTATION_MASK (0x1 << 1)
+#define S3D_ORIENTATION_PORTRAIT 0x0
+#define S3D_ORIENTATION_LANDSCAPE 0x1
+#define EOTP_DISABLED (1 << 0)
+
/* bits 31:0 */
#define _MIPIA_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb084)
#define _MIPIC_DBI_BW_CTRL(dev_priv->mipi_mmio_base + 0xb884)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 12/13] drm/i915/icl: Define TRANS_DSI_FUNC_CONF register
On Tue, 10 Jul 2018, Madhav Chauhan wrote:
> This patch defines transcoder function configuration
> registers and its bitfields for both DSI ports.
> Used while programming/enabling DSI transcoder.
>
> Signed-off-by: Madhav Chauhan
> ---
> drivers/gpu/drm/i915/i915_reg.h | 47
> +
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 62bc76e..71ce6ba 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10134,6 +10134,53 @@ enum skl_power_gate {
> #define TA_SURE_TIME(x) (x << 16)
> #define TA_SURE_TIME_MASK (0x1f << 16)
>
> +/* DSI transcoder configuration */
> +#define _DSI_TRANS_FUNC_CONF_0 0x6b030
> +#define _DSI_TRANS_FUNC_CONF_1 0x6b830
> +#define DSI_TRANS_FUNC_CONF(tc) _MMIO_DSI(tc, \
> + _DSI_TRANS_FUNC_CONF_0,\
> + _DSI_TRANS_FUNC_CONF_1)
> +#define OP_MODE(x) (x << 28)
> +#define OP_MODE_MASK(0x3 << 28)
> +#define CMD_MODE_NO_GATE0x0
> +#define CMD_MODE_TE_GATE0x1
> +#define VIDEO_MODE_SYNC_EVENT 0x2
> +#define VIDEO_MODE_SYNC_PULSE 0x3
The convention is to define macros for field values that you can OR
directly in place instead of requiring a shift. Please stick to the
conventions. Use _SHIFT and _MASK.
We can debate the relative merits of both approaches at some point, but
this is not the time.
BR,
Jani.
> +#define LINK_READY (1 << 20)
> +#define PIX_FMT(x) (x << 16)
> +#define PIX_FMT_MASK(0x3 << 16)
> +#define PIX_FMT_RGB565 0x0
> +#define PIX_FMT_RGB666_PACKED 0x1
> +#define PIX_FMT_RGB666_LOOSE0x2
> +#define PIX_FMT_RGB888 0x3
> +#define PIX_FMT_RGB101010 0x4
> +#define PIX_FMT_RGB121212 0x5
> +#define PIX_FMT_COMPRESSED 0x6
> +#define BGR_TRANSMISSION(1 << 15)
> +#define PIX_VIRT_CHAN(x)(x << 12)
> +#define PIX_VIRT_CHAN_MASK (0x3 << 12)
> +#define PIX_BUF_THRESHOLD(x)((x & 0x3) << 10)
> +#define PIX_BUF_THRESHOLD_MASK (0x3 << 10)
> +#define PIX_BUF_THRESHOLD_1_4 0x0
> +#define PIX_BUF_THRESHOLD_1_2 0x1
> +#define PIX_BUF_THRESHOLD_3_4 0x2
> +#define PIX_BUF_THRESHOLD_FULL 0x3
> +#define CONTINUOUS_CLK(x) (x << 8)
> +#define CONTINUOUS_CLK_MASK (0x3 << 8)
> +#define CLK_ENTER_LP_AFTER_DATA 0x0
> +#define CLK_HS_OR_LP0x2
> +#define CLK_HS_CONTINUOUS 0x3
> +#define LINK_CALIBRATION(x) (x << 4)
> +#define LINK_CALIBRATION_MASK (0x3 << 4)
> +#define CALIBRATION_DISABLED0x0
> +#define CALIBRATION_ENABLED_INITIAL_ONLY0x2
> +#define CALIBRATION_ENABLED_INITIAL_PERIODIC0x3
> +#define S3D_ORIENTATION(x) (x << 1)
> +#define S3D_ORIENTATION_MASK(0x1 << 1)
> +#define S3D_ORIENTATION_PORTRAIT0x0
> +#define S3D_ORIENTATION_LANDSCAPE 0x1
> +#define EOTP_DISABLED (1 << 0)
> +
> /* bits 31:0 */
> #define _MIPIA_DBI_BW_CTRL (dev_priv->mipi_mmio_base + 0xb084)
> #define _MIPIC_DBI_BW_CTRL (dev_priv->mipi_mmio_base + 0xb884)
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 12/13] drm/i915/icl: Define TRANS_DSI_FUNC_CONF register
This patch defines transcoder function configuration
registers and its bitfields for both DSI ports.
Used while programming/enabling DSI transcoder.
Signed-off-by: Madhav Chauhan
---
drivers/gpu/drm/i915/i915_reg.h | 47 +
1 file changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 62bc76e..71ce6ba 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10134,6 +10134,53 @@ enum skl_power_gate {
#define TA_SURE_TIME(x) (x << 16)
#define TA_SURE_TIME_MASK (0x1f << 16)
+/* DSI transcoder configuration */
+#define _DSI_TRANS_FUNC_CONF_0 0x6b030
+#define _DSI_TRANS_FUNC_CONF_1 0x6b830
+#define DSI_TRANS_FUNC_CONF(tc)_MMIO_DSI(tc, \
+ _DSI_TRANS_FUNC_CONF_0,\
+ _DSI_TRANS_FUNC_CONF_1)
+#define OP_MODE(x)(x << 28)
+#define OP_MODE_MASK (0x3 << 28)
+#define CMD_MODE_NO_GATE 0x0
+#define CMD_MODE_TE_GATE 0x1
+#define VIDEO_MODE_SYNC_EVENT 0x2
+#define VIDEO_MODE_SYNC_PULSE 0x3
+#define LINK_READY(1 << 20)
+#define PIX_FMT(x)(x << 16)
+#define PIX_FMT_MASK (0x3 << 16)
+#define PIX_FMT_RGB5650x0
+#define PIX_FMT_RGB666_PACKED 0x1
+#define PIX_FMT_RGB666_LOOSE 0x2
+#define PIX_FMT_RGB8880x3
+#define PIX_FMT_RGB101010 0x4
+#define PIX_FMT_RGB121212 0x5
+#define PIX_FMT_COMPRESSED0x6
+#define BGR_TRANSMISSION (1 << 15)
+#define PIX_VIRT_CHAN(x) (x << 12)
+#define PIX_VIRT_CHAN_MASK(0x3 << 12)
+#define PIX_BUF_THRESHOLD(x) ((x & 0x3) << 10)
+#define PIX_BUF_THRESHOLD_MASK(0x3 << 10)
+#define PIX_BUF_THRESHOLD_1_4 0x0
+#define PIX_BUF_THRESHOLD_1_2 0x1
+#define PIX_BUF_THRESHOLD_3_4 0x2
+#define PIX_BUF_THRESHOLD_FULL0x3
+#define CONTINUOUS_CLK(x) (x << 8)
+#define CONTINUOUS_CLK_MASK (0x3 << 8)
+#define CLK_ENTER_LP_AFTER_DATA 0x0
+#define CLK_HS_OR_LP 0x2
+#define CLK_HS_CONTINUOUS 0x3
+#define LINK_CALIBRATION(x) (x << 4)
+#define LINK_CALIBRATION_MASK (0x3 << 4)
+#define CALIBRATION_DISABLED 0x0
+#define CALIBRATION_ENABLED_INITIAL_ONLY 0x2
+#define CALIBRATION_ENABLED_INITIAL_PERIODIC 0x3
+#define S3D_ORIENTATION(x)(x << 1)
+#define S3D_ORIENTATION_MASK (0x1 << 1)
+#define S3D_ORIENTATION_PORTRAIT 0x0
+#define S3D_ORIENTATION_LANDSCAPE 0x1
+#define EOTP_DISABLED (1 << 0)
+
/* bits 31:0 */
#define _MIPIA_DBI_BW_CTRL (dev_priv->mipi_mmio_base + 0xb084)
#define _MIPIC_DBI_BW_CTRL (dev_priv->mipi_mmio_base + 0xb884)
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
