Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Fri, May 30, 2014 at 09:05:41AM +0100, Sharma, Shashank wrote: > From: Sharma, Shashank > Sent: Thursday, May 22, 2014 5:02 PM > To: intel-gfx@lists.freedesktop.org; Lespiau, Damien; > ville.syrj...@linux.intel.com; Vetter, Daniel > Cc: Kumar, Shobhit; Sharma, Shashank > Subject: [PATCH 2/2] drm/i915: Change Mipi register definitions > > Re-define MIPI register definitions in such a way that most of the existing > DSI code can be re-used for future platforms. Register definitions are > re-written using MMIO offset variable, so that without changing the existing > sequence, same code can be generically applied. > > V3: Addressing review comments by Damien and Ville, added follwing changes: > 1. Replaced _PIPE with _TRANSCODER call, no branching added. > 2. Removed all the un-necessary formatting changes from previous patch. Ooops, I noticed that "oh, he's doing two things in the same patch" but forgot to actually send any mail. So here it is. While the patch looks correct, we try really hard to follow "1 patch, 1 change" esp. when an explanation is needed for each change. I see two changes here: 1/ the mipi_mmio_base change 2/ the _PIPE->_TRANSCODER change This commit should only contain 1/. I would do separate commit for 2, with an explanation. Something like: drm/i915: Use the MIPI transcoder to index MIPI registers Conceptually, the MIPI registers are addressed by the MIPI transcoder index, not the pipe. It doesn't matter right now, because there's a 1:1 relationship between pipes and MIPI transcoders, but that change allows us to break that link in the future. Could you also not reindent the _TRANSCODER() to < 80 chars, I think in that case it removes a bit of readability. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
Gentle reminder for review.
Regards
Shashank
-Original Message-
From: Sharma, Shashank
Sent: Thursday, May 22, 2014 5:02 PM
To: intel-gfx@lists.freedesktop.org; Lespiau, Damien;
ville.syrj...@linux.intel.com; Vetter, Daniel
Cc: Kumar, Shobhit; Sharma, Shashank
Subject: [PATCH 2/2] drm/i915: Change Mipi register definitions
Re-define MIPI register definitions in such a way that most of the existing DSI
code can be re-used for future platforms. Register definitions are re-written
using MMIO offset variable, so that without changing the existing sequence,
same code can be generically applied.
V3: Addressing review comments by Damien and Ville, added follwing changes:
1. Replaced _PIPE with _TRANSCODER call, no branching added.
2. Removed all the un-necessary formatting changes from previous patch.
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/i915_reg.h | 344 ++-
1 file changed, 196 insertions(+), 148 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..38de0e9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5659,7 +5659,8 @@ enum punit_power_well {
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
-#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
_MIPIB_PORT_CTRL)
+#define MIPI_PORT_CTRL(tc) _TRANSCODER(tc, \
+ _MIPIA_PORT_CTRL, _MIPIB_PORT_CTRL)
#define DPI_ENABLE(1 << 31) /* A + B */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
@@ -5701,18 +5702,20 @@ enum punit_power_well {
#define _MIPIA_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61194)
#define _MIPIB_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61704)
-#define MIPI_TEARING_CTRL(pipe)_PIPE(pipe,
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
+#define MIPI_TEARING_CTRL(tc) _TRANSCODER(tc, \
+ _MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
#define TEARING_EFFECT_DELAY_SHIFT0
#define TEARING_EFFECT_DELAY_MASK (0x << 0)
/* XXX: all bits reserved */
-#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
+#define _MIPIA_AUTOPWG (dev_priv->mipi_mmio_base + 0x611a0)
/* MIPI DSI Controller and D-PHY registers */
-#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE + 0xb000)
-#define _MIPIB_DEVICE_READY(VLV_DISPLAY_BASE + 0xb800)
-#define MIPI_DEVICE_READY(pipe)_PIPE(pipe,
_MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
+#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb000)
+#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb800)
+#define MIPI_DEVICE_READY(tc) _TRANSCODER(tc, \
+ _MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
#define BUS_POSSESSION(1 << 3) /* set
to give bus to receiver */
#define ULPS_STATE_MASK (3 << 1)
#define ULPS_STATE_ENTER (2 << 1)
@@ -5720,12 +5723,14 @@ enum punit_power_well {
#define ULPS_STATE_NORMAL_OPERATION (0 << 1)
#define DEVICE_READY (1 << 0)
-#define _MIPIA_INTR_STAT (VLV_DISPLAY_BASE + 0xb004)
-#define _MIPIB_INTR_STAT (VLV_DISPLAY_BASE + 0xb804)
-#define MIPI_INTR_STAT(pipe) _PIPE(pipe, _MIPIA_INTR_STAT,
_MIPIB_INTR_STAT)
-#define _MIPIA_INTR_EN (VLV_DISPLAY_BASE + 0xb008)
-#define _MIPIB_INTR_EN (VLV_DISPLAY_BASE + 0xb808)
-#define MIPI_INTR_EN(pipe) _PIPE(pipe, _MIPIA_INTR_EN,
_MIPIB_INTR_EN)
+#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + 0xb004)
+#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + 0xb804)
+#define MIPI_INTR_STAT(tc) _TRANSCODER(tc, \
+ _MIPIA_INTR_STAT, _MIPIB_INTR_STAT)
+#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base + 0xb008)
+#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base + 0xb808)
+#define MIPI_INTR_EN(tc) _TRANSCODER(tc, \
+ _MIPIA_INTR_EN, _MIPIB_INTR_EN)
#define TEARING_EFFECT(1 << 31)
#define SPL_PKT_SENT_INTERRUPT(1 << 30)
#define GEN_READ_DATA_AVAIL (1 << 29)
@@ -5759,9 +5764,10 @@ enum punit_power_well {
#define RXSOT_SYNC_ERROR (1 << 1)
#define RXSOT_ERROR
[Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
Re-define MIPI register definitions in such a way that most of
the existing DSI code can be re-used for future platforms. Register
definitions are re-written using MMIO offset variable, so that without
changing the existing sequence, same code can be generically applied.
V3: Addressing review comments by Damien and Ville, added follwing changes:
1. Replaced _PIPE with _TRANSCODER call, no branching added.
2. Removed all the un-necessary formatting changes from previous patch.
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/i915_reg.h | 344 ++-
1 file changed, 196 insertions(+), 148 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..38de0e9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5659,7 +5659,8 @@ enum punit_power_well {
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
-#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
_MIPIB_PORT_CTRL)
+#define MIPI_PORT_CTRL(tc) _TRANSCODER(tc, \
+ _MIPIA_PORT_CTRL, _MIPIB_PORT_CTRL)
#define DPI_ENABLE(1 << 31) /* A + B */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
@@ -5701,18 +5702,20 @@ enum punit_power_well {
#define _MIPIA_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61194)
#define _MIPIB_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61704)
-#define MIPI_TEARING_CTRL(pipe)_PIPE(pipe,
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
+#define MIPI_TEARING_CTRL(tc) _TRANSCODER(tc, \
+ _MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
#define TEARING_EFFECT_DELAY_SHIFT0
#define TEARING_EFFECT_DELAY_MASK (0x << 0)
/* XXX: all bits reserved */
-#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
+#define _MIPIA_AUTOPWG (dev_priv->mipi_mmio_base + 0x611a0)
/* MIPI DSI Controller and D-PHY registers */
-#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE + 0xb000)
-#define _MIPIB_DEVICE_READY(VLV_DISPLAY_BASE + 0xb800)
-#define MIPI_DEVICE_READY(pipe)_PIPE(pipe,
_MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
+#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb000)
+#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb800)
+#define MIPI_DEVICE_READY(tc) _TRANSCODER(tc, \
+ _MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
#define BUS_POSSESSION(1 << 3) /* set
to give bus to receiver */
#define ULPS_STATE_MASK (3 << 1)
#define ULPS_STATE_ENTER (2 << 1)
@@ -5720,12 +5723,14 @@ enum punit_power_well {
#define ULPS_STATE_NORMAL_OPERATION (0 << 1)
#define DEVICE_READY (1 << 0)
-#define _MIPIA_INTR_STAT (VLV_DISPLAY_BASE + 0xb004)
-#define _MIPIB_INTR_STAT (VLV_DISPLAY_BASE + 0xb804)
-#define MIPI_INTR_STAT(pipe) _PIPE(pipe, _MIPIA_INTR_STAT,
_MIPIB_INTR_STAT)
-#define _MIPIA_INTR_EN (VLV_DISPLAY_BASE + 0xb008)
-#define _MIPIB_INTR_EN (VLV_DISPLAY_BASE + 0xb808)
-#define MIPI_INTR_EN(pipe) _PIPE(pipe, _MIPIA_INTR_EN,
_MIPIB_INTR_EN)
+#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + 0xb004)
+#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + 0xb804)
+#define MIPI_INTR_STAT(tc) _TRANSCODER(tc, \
+ _MIPIA_INTR_STAT, _MIPIB_INTR_STAT)
+#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base + 0xb008)
+#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base + 0xb808)
+#define MIPI_INTR_EN(tc) _TRANSCODER(tc, \
+ _MIPIA_INTR_EN, _MIPIB_INTR_EN)
#define TEARING_EFFECT(1 << 31)
#define SPL_PKT_SENT_INTERRUPT(1 << 30)
#define GEN_READ_DATA_AVAIL (1 << 29)
@@ -5759,9 +5764,10 @@ enum punit_power_well {
#define RXSOT_SYNC_ERROR (1 << 1)
#define RXSOT_ERROR (1 << 0)
-#define _MIPIA_DSI_FUNC_PRG(VLV_DISPLAY_BASE + 0xb00c)
-#define _MIPIB_DSI_FUNC_PRG(VLV_DISPLAY_BASE + 0xb80c)
-#define MIPI_DSI_FUNC_PRG(pipe)_PIPE(pipe,
_MIPIA_DSI_FUNC_PRG, _MIPIB_DSI_FUNC_PRG)
+#define _MIPIA_DSI_FUNC_PRG(dev_priv->mipi_mmio_base +
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Wed, May 21, 2014 at 06:35:12PM +0300, Ville Syrjälä wrote: > > + > > +/* VLV port control */ > > #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190) > > #define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700) > > #define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL, > > _MIPIB_PORT_CTRL) > > Why isn't mipi_mmio_base used here? Does the register not need the new > offset? I htink it would still be cleaner to use the mipi_mmio_offset > for all the MIPI registers. On VLV, this register (a at least one other) isn't part of the MIPI IP block address space (base + 0xbxxx) so shouldn't be defined as mipi_mmio_base + offset. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
Thanks for pointing this out.
I will correct all formatting stuff and re-send patch.
Regards
Shashank
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Wednesday, May 21, 2014 9:05 PM
To: Sharma, Shashank
Cc: Lespiau, Damien; Vetter, Daniel; intel-gfx@lists.freedesktop.org; Nikula,
Jani
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Wed, May 21, 2014 at 08:56:59PM +0530, Shashank Sharma wrote:
> Re-define MIPI register definitions in such a way that most of the
> existing DSI code can be re-used for future platforms. Register
> definitions are re-written using MMIO offset variable, so that without
> changing the existing sequence, same code can be generically applied.
>
> V2: Addressing review comments by Damien, added follwing changes:
> 1. Re-defined MIPI_DSI_FUNC_PRG using _PIPE macro, to remove
> branching.
> 2. Re-written _MIPIB_DSI_FUNC_PRG and _MIPIA_DSI_FUNC_PRG
> in single line.
> Signed-off-by: Shashank Sharma
Sorry but this patch is a mess. Way too many pointless formatting changes in
there.
> ---
> drivers/gpu/drm/i915/i915_reg.h | 689
> +++
> 1 file changed, 416 insertions(+), 273 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index c12a858..50d5e89 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5655,20 +5655,23 @@ enum punit_power_well { #define
> PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
> _PIPE_B_CSC_POSTOFF_ME) #define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe,
> _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
>
> -/* VLV MIPI registers */
>
> +/* MIPI registers */
This change is not needed.
> +
> +/* VLV port control */
> #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
> #define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
> #define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
> _MIPIB_PORT_CTRL)
Why isn't mipi_mmio_base used here? Does the register not need the new offset?
I htink it would still be cleaner to use the mipi_mmio_offset for all the MIPI
registers.
> -#define DPI_ENABLE (1 << 31) /* A + B */
> +
> +#define DPI_ENABLE (1 << 31)
Not needed.
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK(0xf << 27)
> #define DUAL_LINK_MODE_MASK (1 << 26)
> #define DUAL_LINK_MODE_FRONT_BACK (0 << 26)
> #define DUAL_LINK_MODE_PIXEL_ALTERNATIVE(1 << 26)
> -#define DITHERING_ENABLE(1 << 25) /* A + B */
> +#define DITHERING_ENABLE(1 << 25)
> #define FLOPPED_HSTX(1 << 23)
> -#define DE_INVERT (1 << 19) /* XXX */
> +#define DE_INVERT (1 << 19)
More unneeded comment changes.
> #define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
> #define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf << 18)
> #define AFE_LATCHOUT(1 << 17)
> @@ -5699,33 +5702,46 @@ enum punit_power_well {
> #define LANE_CONFIGURATION_DUAL_LINK_A (1 << 0)
> #define LANE_CONFIGURATION_DUAL_LINK_B (2 << 0)
>
> +/* VLV tearing effect control */
> #define _MIPIA_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61194)
> #define _MIPIB_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61704)
> #define MIPI_TEARING_CTRL(pipe) _PIPE(pipe,
> _MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
> -#define TEARING_EFFECT_DELAY_SHIFT 0
> -#define TEARING_EFFECT_DELAY_MASK (0x << 0)
> +#define TEARING_EFFECT_DELAY_SHIFT 0
> +#define TEARING_EFFECT_DELAY_MASK(0x << 0)
Bad formatting change.
etc. Did you run this through some autoformatting tool or something?
Please don't do that. A simple sed job should be all that's needed for
mipi_mmio_offset.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Wed, May 21, 2014 at 08:56:59PM +0530, Shashank Sharma wrote:
> Re-define MIPI register definitions in such a way that most of
> the existing DSI code can be re-used for future platforms. Register
> definitions are re-written using MMIO offset variable, so that without
> changing the existing sequence, same code can be generically applied.
>
> V2: Addressing review comments by Damien, added follwing changes:
> 1. Re-defined MIPI_DSI_FUNC_PRG using _PIPE macro, to remove
> branching.
> 2. Re-written _MIPIB_DSI_FUNC_PRG and _MIPIA_DSI_FUNC_PRG
> in single line.
> Signed-off-by: Shashank Sharma
Sorry but this patch is a mess. Way too many pointless formatting
changes in there.
> ---
> drivers/gpu/drm/i915/i915_reg.h | 689
> +++
> 1 file changed, 416 insertions(+), 273 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c12a858..50d5e89 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5655,20 +5655,23 @@ enum punit_power_well {
> #define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
> _PIPE_B_CSC_POSTOFF_ME)
> #define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO,
> _PIPE_B_CSC_POSTOFF_LO)
>
> -/* VLV MIPI registers */
>
> +/* MIPI registers */
This change is not needed.
> +
> +/* VLV port control */
> #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
> #define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
> #define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
> _MIPIB_PORT_CTRL)
Why isn't mipi_mmio_base used here? Does the register not need the new
offset? I htink it would still be cleaner to use the mipi_mmio_offset
for all the MIPI registers.
> -#define DPI_ENABLE (1 << 31) /* A + B */
> +
> +#define DPI_ENABLE (1 << 31)
Not needed.
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK(0xf << 27)
> #define DUAL_LINK_MODE_MASK (1 << 26)
> #define DUAL_LINK_MODE_FRONT_BACK (0 << 26)
> #define DUAL_LINK_MODE_PIXEL_ALTERNATIVE(1 << 26)
> -#define DITHERING_ENABLE(1 << 25) /* A + B */
> +#define DITHERING_ENABLE(1 << 25)
> #define FLOPPED_HSTX(1 << 23)
> -#define DE_INVERT (1 << 19) /* XXX */
> +#define DE_INVERT (1 << 19)
More unneeded comment changes.
> #define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
> #define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf << 18)
> #define AFE_LATCHOUT(1 << 17)
> @@ -5699,33 +5702,46 @@ enum punit_power_well {
> #define LANE_CONFIGURATION_DUAL_LINK_A (1 << 0)
> #define LANE_CONFIGURATION_DUAL_LINK_B (2 << 0)
>
> +/* VLV tearing effect control */
> #define _MIPIA_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61194)
> #define _MIPIB_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61704)
> #define MIPI_TEARING_CTRL(pipe) _PIPE(pipe,
> _MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
> -#define TEARING_EFFECT_DELAY_SHIFT 0
> -#define TEARING_EFFECT_DELAY_MASK (0x << 0)
> +#define TEARING_EFFECT_DELAY_SHIFT 0
> +#define TEARING_EFFECT_DELAY_MASK(0x << 0)
Bad formatting change.
etc. Did you run this through some autoformatting tool or something?
Please don't do that. A simple sed job should be all that's needed
for mipi_mmio_offset.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
Re-define MIPI register definitions in such a way that most of
the existing DSI code can be re-used for future platforms. Register
definitions are re-written using MMIO offset variable, so that without
changing the existing sequence, same code can be generically applied.
V2: Addressing review comments by Damien, added follwing changes:
1. Re-defined MIPI_DSI_FUNC_PRG using _PIPE macro, to remove
branching.
2. Re-written _MIPIB_DSI_FUNC_PRG and _MIPIA_DSI_FUNC_PRG
in single line.
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/i915_reg.h | 689 +++
1 file changed, 416 insertions(+), 273 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..50d5e89 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5655,20 +5655,23 @@ enum punit_power_well {
#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
_PIPE_B_CSC_POSTOFF_ME)
#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO,
_PIPE_B_CSC_POSTOFF_LO)
-/* VLV MIPI registers */
+/* MIPI registers */
+
+/* VLV port control */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
_MIPIB_PORT_CTRL)
-#define DPI_ENABLE(1 << 31) /* A + B */
+
+#define DPI_ENABLE(1 << 31)
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
#define DUAL_LINK_MODE_MASK (1 << 26)
#define DUAL_LINK_MODE_FRONT_BACK (0 << 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26)
-#define DITHERING_ENABLE (1 << 25) /* A + B */
+#define DITHERING_ENABLE (1 << 25)
#define FLOPPED_HSTX (1 << 23)
-#define DE_INVERT (1 << 19) /* XXX */
+#define DE_INVERT (1 << 19)
#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
#define MIPIA_FLISDSI_DELAY_COUNT_MASK(0xf << 18)
#define AFE_LATCHOUT (1 << 17)
@@ -5699,33 +5702,46 @@ enum punit_power_well {
#define LANE_CONFIGURATION_DUAL_LINK_A(1 << 0)
#define LANE_CONFIGURATION_DUAL_LINK_B(2 << 0)
+/* VLV tearing effect control */
#define _MIPIA_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61194)
#define _MIPIB_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61704)
#define MIPI_TEARING_CTRL(pipe)_PIPE(pipe,
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
-#define TEARING_EFFECT_DELAY_SHIFT0
-#define TEARING_EFFECT_DELAY_MASK (0x << 0)
+#define TEARING_EFFECT_DELAY_SHIFT 0
+#define TEARING_EFFECT_DELAY_MASK (0x << 0)
-/* XXX: all bits reserved */
-#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
+/* VLV: all bits reserved */
+#define _MIPIA_AUTOPWG (dev_priv->mipi_mmio_base + \
+ 0x611a0)
/* MIPI DSI Controller and D-PHY registers */
+#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + \
+ 0xb000)
+#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + \
+ 0xb800)
+#define MIPI_DEVICE_READY(check) (!check ? \
+ _MIPIA_DEVICE_READY : _MIPIB_DEVICE_READY)
+
+#define BUS_POSSESSION(1 << 3) /* set to give bus to receiver
*/
+#define ULPS_STATE_MASK (3 << 1)
+#define ULPS_STATE_ENTER (2 << 1)
+#define ULPS_STATE_EXIT (1 << 1)
+#define ULPS_STATE_NORMAL_OPERATION (0 << 1)
+#define DEVICE_READY (1 << 0)
+
+#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + \
+ 0xb004)
+#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + \
+ 0xb804)
+#define MIPI_INTR_STAT(check) (!check ? \
+ _MIPIA_INTR_STAT : _MIPIB_INTR_STAT)
+
+#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base + \
+ 0xb008)
+#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base + \
+ 0xb808)
+#define MIPI_INTR_EN(check)(!check ? \
+ _MIPIA_INTR_EN : _MIPIB_INTR_EN)
-#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE + 0xb000)
-#define _MIPIB_DEVICE_REA
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
On Mon, May 19, 2014 at 08:54:04PM +0530, Shashank Sharma wrote: > Re-define MIPI register definitions in such a way that most of > the existing DSI code can be re-used for future platforms. Register > definitions are re-written using MMIO offset variable, so that without > changing the existing sequence, same code can be generically applied. > > Signed-off-by: Shashank Sharma Two little things: * You changed the _PIPE() macro to something with a jump, can we please not introduce branches here? might as well keep the _PIPE() macro, even if _TRANSCODER() would be slightly better (but strickly equivalent * You've cut everything to be < 80 chars. I really think that's one of the cases where it's worse, ie: #define _MIPIB_DSI_FUNC_PRG (dev_priv->mipi_mmio_base + \ 0xb80c) #define MIPI_DSI_FUNC_PRG(check)(!check ? \ _MIPIA_DSI_FUNC_PRG : _MIPIB_DSI_FUNC_PRG) Vs #define _MIPIB_DSI_FUNC_PRG (dev_priv->mipi_mmio_base + 0xb80c) #define MIPI_DSI_FUNC_PRG(pipe) _PIPE(pipe, _MIPIA_DSI_FUNC_PRG, _MIPIB_DSI_FUNC_PRG) Can we not do that? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions
Re-define MIPI register definitions in such a way that most of
the existing DSI code can be re-used for future platforms. Register
definitions are re-written using MMIO offset variable, so that without
changing the existing sequence, same code can be generically applied.
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/i915_reg.h | 691 +++
1 file changed, 418 insertions(+), 273 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..ba71e27 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5655,20 +5655,23 @@ enum punit_power_well {
#define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME,
_PIPE_B_CSC_POSTOFF_ME)
#define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO,
_PIPE_B_CSC_POSTOFF_LO)
-/* VLV MIPI registers */
+/* MIPI registers */
+
+/* VLV port control */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
#define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL,
_MIPIB_PORT_CTRL)
-#define DPI_ENABLE(1 << 31) /* A + B */
+
+#define DPI_ENABLE(1 << 31)
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
#define DUAL_LINK_MODE_MASK (1 << 26)
#define DUAL_LINK_MODE_FRONT_BACK (0 << 26)
#define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26)
-#define DITHERING_ENABLE (1 << 25) /* A + B */
+#define DITHERING_ENABLE (1 << 25)
#define FLOPPED_HSTX (1 << 23)
-#define DE_INVERT (1 << 19) /* XXX */
+#define DE_INVERT (1 << 19)
#define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18
#define MIPIA_FLISDSI_DELAY_COUNT_MASK(0xf << 18)
#define AFE_LATCHOUT (1 << 17)
@@ -5699,33 +5702,46 @@ enum punit_power_well {
#define LANE_CONFIGURATION_DUAL_LINK_A(1 << 0)
#define LANE_CONFIGURATION_DUAL_LINK_B(2 << 0)
+/* VLV tearing effect control */
#define _MIPIA_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61194)
#define _MIPIB_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61704)
#define MIPI_TEARING_CTRL(pipe)_PIPE(pipe,
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
-#define TEARING_EFFECT_DELAY_SHIFT0
-#define TEARING_EFFECT_DELAY_MASK (0x << 0)
+#define TEARING_EFFECT_DELAY_SHIFT 0
+#define TEARING_EFFECT_DELAY_MASK (0x << 0)
-/* XXX: all bits reserved */
-#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
+/* VLV: all bits reserved */
+#define _MIPIA_AUTOPWG (dev_priv->mipi_mmio_base + \
+ 0x611a0)
/* MIPI DSI Controller and D-PHY registers */
+#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + \
+ 0xb000)
+#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + \
+ 0xb800)
+#define MIPI_DEVICE_READY(check) (!check ? \
+ _MIPIA_DEVICE_READY : _MIPIB_DEVICE_READY)
+
+#define BUS_POSSESSION(1 << 3) /* set to give bus to receiver
*/
+#define ULPS_STATE_MASK (3 << 1)
+#define ULPS_STATE_ENTER (2 << 1)
+#define ULPS_STATE_EXIT (1 << 1)
+#define ULPS_STATE_NORMAL_OPERATION (0 << 1)
+#define DEVICE_READY (1 << 0)
+
+#define _MIPIA_INTR_STAT (dev_priv->mipi_mmio_base + \
+ 0xb004)
+#define _MIPIB_INTR_STAT (dev_priv->mipi_mmio_base + \
+ 0xb804)
+#define MIPI_INTR_STAT(check) (!check ? \
+ _MIPIA_INTR_STAT : _MIPIB_INTR_STAT)
+
+#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base + \
+ 0xb008)
+#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base + \
+ 0xb808)
+#define MIPI_INTR_EN(check)(!check ? \
+ _MIPIA_INTR_EN : _MIPIB_INTR_EN)
-#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE + 0xb000)
-#define _MIPIB_DEVICE_READY(VLV_DISPLAY_BASE + 0xb800)
-#define MIPI_DEVICE_READY(pipe)_PIPE(pipe,
_MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
-#define BUS_POSSESSION(1 << 3) /*
