* [PATCH 00/12] *** MIPI DSI Support for BXT ***
@ 2015-05-22 16:05 Uma Shankar
2015-05-22 16:05 ` [PATCH 01/12] drm/i915/bxt: Initialize MIPI for BXT Uma Shankar
` (11 more replies)
0 siblings, 12 replies; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
This patch series adds support for MIPI DSI for BXT platform.
Support for VBT v3 sequence parsing and programming is needed
for panel, backlight enable and control. The same will be added
as part of a different patch series.
Shashank Sharma (10):
drm/i915/bxt: Initialize MIPI for BXT
drm/i915/bxt: Enable BXT DSI PLL
drm/i915/bxt: Disable DSI PLL for BXT
drm/i915/bxt: DSI prepare changes for BXT
drm/i915/bxt: DSI encoder support in CRTC modeset
drm/i915/bxt: DSI enable for BXT
drm/i915/bxt: Program Tx Rx and Dphy clocks
drm/i915/bxt: DSI disable and post-disable
drm/i915/bxt: get_hw_state for BXT
drm/i915/bxt: get DSI pixelclock
Sunil Kamath (1):
drm/i915/bxt: Modify BXT BLC according to VBT changes
Uma Shankar (1):
drm/i915/bxt: Remove DSP CLK_GATE programming for BXT
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 140 +++++++++++++-
drivers/gpu/drm/i915/intel_ddi.c | 53 ++++-
drivers/gpu/drm/i915/intel_display.c | 9 +-
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_dsi.c | 341 +++++++++++++++++++++++++--------
drivers/gpu/drm/i915/intel_dsi.h | 8 +-
drivers/gpu/drm/i915/intel_dsi_pll.c | 227 +++++++++++++++++++++-
drivers/gpu/drm/i915/intel_opregion.c | 1 +
drivers/gpu/drm/i915/intel_panel.c | 100 ++++++++--
10 files changed, 765 insertions(+), 117 deletions(-)
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/12] drm/i915/bxt: Initialize MIPI for BXT
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
@ 2015-05-22 16:05 ` Uma Shankar
2015-05-22 16:05 ` [PATCH 02/12] drm/i915/bxt: Enable BXT DSI PLL Uma Shankar
` (10 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
This patch contains following changes:
1. Add BXT MIPI display address base.
2. Call dsi_init from display_setup function.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 3 +++
drivers/gpu/drm/i915/intel_dsi.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 84af255..46ef269 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1643,6 +1643,7 @@ enum skl_disp_power_wells {
#define VLV_DISPLAY_BASE 0x180000
#define VLV_MIPI_BASE VLV_DISPLAY_BASE
+#define BXT_MIPI_BASE 0x60000
#define VLV_GU_CTL0 (VLV_DISPLAY_BASE + 0x2030)
#define VLV_GU_CTL1 (VLV_DISPLAY_BASE + 0x2034)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9d2d6fb..c30bfd4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13706,6 +13706,9 @@ static void intel_setup_outputs(struct drm_device *dev)
* DDI_BUF_CTL_A or SFUSE_STRAP registers, find another way to
* detect the ports.
*/
+ /* Initialize MIPI for BXT */
+ intel_dsi_init(dev);
+
intel_ddi_init(dev, PORT_A);
intel_ddi_init(dev, PORT_B);
intel_ddi_init(dev, PORT_C);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 5196642..2419929e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -998,6 +998,8 @@ void intel_dsi_init(struct drm_device *dev)
if (IS_VALLEYVIEW(dev)) {
dev_priv->mipi_mmio_base = VLV_MIPI_BASE;
+ } else if (IS_BROXTON(dev)) {
+ dev_priv->mipi_mmio_base = BXT_MIPI_BASE;
} else {
DRM_ERROR("Unsupported Mipi device to reg base");
return;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/12] drm/i915/bxt: Enable BXT DSI PLL
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
2015-05-22 16:05 ` [PATCH 01/12] drm/i915/bxt: Initialize MIPI for BXT Uma Shankar
@ 2015-05-22 16:05 ` Uma Shankar
2015-05-25 16:10 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 03/12] drm/i915/bxt: Disable DSI PLL for BXT Uma Shankar
` (9 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
This patch adds new functions for BXT clock and PLL programming.
They are:
1. configure_dsi_pll for BXT.
This function does the basic math and generates the divider ratio
based on requested pixclock, and program clock registers.
2. enable_dsi_pll function.
This function programs the calculated clock values on the PLL.
3. intel_enable_dsi_pll
Wrapper function to use same code for multiple platforms. It checks the
platform and calls appropriate core pll enable function.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 35 ++++++++++++++
drivers/gpu/drm/i915/intel_dsi.c | 2 +-
drivers/gpu/drm/i915/intel_dsi.h | 2 +-
drivers/gpu/drm/i915/intel_dsi_pll.c | 85 +++++++++++++++++++++++++++++++++-
4 files changed, 121 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 46ef269..b63bdce 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7352,6 +7352,41 @@ enum skl_disp_power_wells {
/* MIPI DSI registers */
+/* BXT DSI PLL Control */
+#define BXT_DSI_PLL_CTL 0x161000
+#define BXT_DSI_PLL_MASK_PVD_RATIO (3 << 16)
+#define BXT_DSI_PLL_PVD_RATIO_1 (1<<16)
+#define BXT_DSIC_16X_BY2 (1 << 10)
+#define BXT_DSIC_16X_BY3 (1 << 11)
+#define BXT_DSIC_16X_BY4 (3 << 10)
+#define BXT_DSIA_16X_BY2 (1 << 8)
+#define BXT_DSIA_16X_BY3 (1 << 9)
+#define BXT_DSIA_16X_BY4 (3 << 8)
+
+#define BXT_DSI_MASK_FREQ_SEL (0xF << 8)
+
+#define BXT_DSI_PLL_RATIO_MAX 0x7D
+#define BXT_DSI_PLL_RATIO_MIN 0x22
+#define BXT_DSI_MASK_PLL_RATIO 0x7F
+#define BXT_REF_CLOCK_KHZ 19500
+
+#define BXT_DSI_PLL_ENABLE 0x46080
+#define BXT_DSI_PLL_DO_ENABLE (1 << 31)
+#define BXT_DSI_PLL_LOCKED (1 << 30)
+
+/* BXT power well control2, for driver */
+#define BXT_PWR_WELL_CTL2 0x45404
+#define BXT_PWR_WELL_2_ENABLE (1 << 31)
+#define BXT_PWR_WELL_2_ENABLED (1 << 30)
+#define BXT_PWR_WELL_1_ENABLE (1 << 29)
+#define BXT_PWR_WELL_1_ENABLED (1 << 28)
+#define BXT_PWR_WELL_ENABLED (BXT_PWR_WELL_1_ENABLED | \
+ BXT_PWR_WELL_2_ENABLED)
+#define GEN9_FUSE_STATUS 0x42000
+#define GEN9_FUSE_PG2_ENABLED (1 << 25)
+#define GEN9_FUSE_PG1_ENABLED (1 << 26)
+#define GEN9_FUSE_PG0_ENABLED (1 << 27)
+
#define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 2419929e..1927546 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -903,8 +903,8 @@ static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
DRM_DEBUG_KMS("\n");
intel_dsi_prepare(encoder);
+ intel_enable_dsi_pll(encoder);
- vlv_enable_dsi_pll(encoder);
}
static enum drm_connector_status
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 2784ac4..20cfcf07 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -121,7 +121,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
return container_of(encoder, struct intel_dsi, base.base);
}
-extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
+extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 9688d99..a58f0a1 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -237,7 +237,7 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, dsi_mnp.dsi_pll_ctrl);
}
-void vlv_enable_dsi_pll(struct intel_encoder *encoder)
+static void vlv_enable_dsi_pll(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
u32 tmp;
@@ -368,3 +368,86 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
return pclk;
}
+
+static bool bxt_configure_dsi_pll(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ u8 dsi_ratio;
+ u32 dsi_clk;
+ u32 val = 0;
+
+ dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
+ intel_dsi->lane_count);
+
+ /*
+ * From clock diagram, to get PLL ratio divider, divide double of DSI
+ * link rate (i.e., 2*8x=16x frequency value) by ref clock. Make sure to
+ * round 'up' the result
+ */
+ dsi_ratio = DIV_ROUND_UP(dsi_clk * 2, BXT_REF_CLOCK_KHZ);
+ if (dsi_ratio < BXT_DSI_PLL_RATIO_MIN ||
+ dsi_ratio > BXT_DSI_PLL_RATIO_MAX) {
+ DRM_ERROR("Cant get a suitable ratio from DSI PLL ratios\n");
+ return false;
+ }
+
+ /*
+ * Program DSI ratio and Select MIPIC and MIPIA PLL output as 8x
+ * Spec says both have to be programmed, even if one is not getting
+ * used. Configure MIPI_CLOCK_CTL dividers in modeset
+ */
+ val = I915_READ(BXT_DSI_PLL_CTL);
+ val &= ~BXT_DSI_PLL_MASK_PVD_RATIO;
+ val &= ~BXT_DSI_MASK_FREQ_SEL;
+ val &= ~BXT_DSI_MASK_PLL_RATIO;
+ val |= (dsi_ratio | BXT_DSIA_16X_BY2 | BXT_DSIC_16X_BY2);
+
+ /* Prog PVD ratio =1 if ratio <= 50 */
+ if (dsi_ratio <= 50) {
+ val &= ~BXT_DSI_PLL_MASK_PVD_RATIO;
+ val |= BXT_DSI_PLL_PVD_RATIO_1;
+ }
+
+ I915_WRITE(BXT_DSI_PLL_CTL, val);
+ return true;
+}
+
+static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ u32 val = 0;
+
+ DRM_DEBUG_KMS("\n");
+
+ /* Configure PLL vales */
+ if (!bxt_configure_dsi_pll(encoder)) {
+ DRM_ERROR("Configure DSI PLL failed, abort PLL enable\n");
+ return;
+ }
+
+ /* Enable DSI PLL */
+ val = I915_READ(BXT_DSI_PLL_ENABLE);
+ val |= BXT_DSI_PLL_DO_ENABLE;
+ I915_WRITE(BXT_DSI_PLL_ENABLE, val);
+
+ /* Timeout and fail if PLL not locked */
+ if (wait_for(I915_READ(BXT_DSI_PLL_ENABLE) & BXT_DSI_PLL_LOCKED, 1)) {
+ DRM_ERROR("DSI PLL not locked\n");
+ return;
+ }
+
+ DRM_DEBUG_KMS("DSI PLL locked\n");
+}
+
+void intel_enable_dsi_pll(struct intel_encoder *encoder)
+{
+ struct drm_device *dev = encoder->base.dev;
+
+ if (IS_VALLEYVIEW(dev))
+ vlv_enable_dsi_pll(encoder);
+ else if (IS_BROXTON(dev))
+ bxt_enable_dsi_pll(encoder);
+ else
+ DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
+}
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 03/12] drm/i915/bxt: Disable DSI PLL for BXT
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
2015-05-22 16:05 ` [PATCH 01/12] drm/i915/bxt: Initialize MIPI for BXT Uma Shankar
2015-05-22 16:05 ` [PATCH 02/12] drm/i915/bxt: Enable BXT DSI PLL Uma Shankar
@ 2015-05-22 16:05 ` Uma Shankar
2015-05-25 16:12 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 04/12] drm/i915/bxt: DSI prepare changes " Uma Shankar
` (8 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
This patch adds two new functions:
- disable_dsi_pll.
BXT DSI disable sequence and registers are
different from previous platforms.
- intel_disable_dsi_pll
wrapper function to re-use the same code for
multiple platforms. It checks platform type and
calls appropriate core pll disable function.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_dsi.c | 2 +-
drivers/gpu/drm/i915/intel_dsi.h | 2 +-
drivers/gpu/drm/i915/intel_dsi_pll.c | 31 ++++++++++++++++++++++++++++++-
3 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 1927546..2c0ea6f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -553,7 +553,7 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
usleep_range(2000, 2500);
}
- vlv_disable_dsi_pll(encoder);
+ intel_disable_dsi_pll(encoder);
}
static void intel_dsi_post_disable(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 20cfcf07..759983e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -122,7 +122,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
}
extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
-extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
+extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index a58f0a1..0cbcf32 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -267,7 +267,7 @@ static void vlv_enable_dsi_pll(struct intel_encoder *encoder)
DRM_DEBUG_KMS("DSI PLL locked\n");
}
-void vlv_disable_dsi_pll(struct intel_encoder *encoder)
+static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
u32 tmp;
@@ -440,6 +440,23 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
DRM_DEBUG_KMS("DSI PLL locked\n");
}
+static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+
+ DRM_DEBUG_KMS("\n");
+
+ /* Disable DSI PLL */
+ I915_WRITE(BXT_DSI_PLL_ENABLE, 0);
+
+ /* Should timeout and fail if not locked after 200 us */
+ if (wait_for((I915_READ(BXT_DSI_PLL_ENABLE)
+ & BXT_DSI_PLL_LOCKED) == 0, 1)) {
+ DRM_ERROR("Disable: DSI PLL not locked\n");
+ return;
+ }
+}
+
void intel_enable_dsi_pll(struct intel_encoder *encoder)
{
struct drm_device *dev = encoder->base.dev;
@@ -451,3 +468,15 @@ void intel_enable_dsi_pll(struct intel_encoder *encoder)
else
DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
}
+
+void intel_disable_dsi_pll(struct intel_encoder *encoder)
+{
+ struct drm_device *dev = encoder->base.dev;
+
+ if (IS_VALLEYVIEW(dev))
+ vlv_disable_dsi_pll(encoder);
+ else if (IS_BROXTON(dev))
+ bxt_disable_dsi_pll(encoder);
+ else
+ DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
+}
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 04/12] drm/i915/bxt: DSI prepare changes for BXT
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (2 preceding siblings ...)
2015-05-22 16:05 ` [PATCH 03/12] drm/i915/bxt: Disable DSI PLL for BXT Uma Shankar
@ 2015-05-22 16:05 ` Uma Shankar
2015-05-25 16:25 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset Uma Shankar
` (7 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
This patch modifies dsi_prepare() function to support the same
modeset prepare sequence for BXT also. Main changes are:
1. BXT port control register is different than VLV.
2. BXT modeset sequence needs vdisplay and hdisplay programmed
for transcoder.
3. BXT can select PIPE for MIPI transcoders.
4. BXT needs to program register MIPI_INIT_COUNT for both the ports,
even if only one is being used.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 22 +++++++++++
drivers/gpu/drm/i915/intel_dsi.c | 79 +++++++++++++++++++++++++++++++++-----
2 files changed, 91 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b63bdce..57c3a48 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7387,6 +7387,22 @@ enum skl_disp_power_wells {
#define GEN9_FUSE_PG1_ENABLED (1 << 26)
#define GEN9_FUSE_PG0_ENABLED (1 << 27)
+/* BXT MIPI mode configure */
+#define _BXT_MIPIA_TRANS_HACTIVE 0x6B0F8
+#define _BXT_MIPIC_TRANS_HACTIVE 0x6B8F8
+#define BXT_MIPI_TRANS_HACTIVE(tc) _TRANSCODER(tc, \
+ _BXT_MIPIA_TRANS_HACTIVE, _BXT_MIPIC_TRANS_HACTIVE)
+
+#define _BXT_MIPIA_TRANS_VACTIVE 0x6B0FC
+#define _BXT_MIPIC_TRANS_VACTIVE 0x6B8FC
+#define BXT_MIPI_TRANS_VACTIVE(tc) _TRANSCODER(tc, \
+ _BXT_MIPIA_TRANS_VACTIVE, _BXT_MIPIC_TRANS_VACTIVE)
+
+#define _BXT_MIPIA_TRANS_VTOTAL 0x6B100
+#define _BXT_MIPIC_TRANS_VTOTAL 0x6B900
+#define BXT_MIPI_TRANS_VTOTAL(tc) _TRANSCODER(tc, \
+ _BXT_MIPIA_TRANS_VTOTAL, _BXT_MIPIC_TRANS_VTOTAL)
+
#define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
@@ -7802,6 +7818,12 @@ enum skl_disp_power_wells {
#define READ_REQUEST_PRIORITY_HIGH (3 << 3)
#define RGB_FLIP_TO_BGR (1 << 2)
+/* BXT has a pipe select field */
+#define BXT_PIPE_SELECT_A (0 << 7)
+#define BXT_PIPE_SELECT_B (1 << 7)
+#define BXT_PIPE_SELECT_C (2 << 7)
+#define BXT_PIPE_SELECT_MASK (7 << 7)
+
#define _MIPIA_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb108)
#define _MIPIC_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb908)
#define MIPI_DATA_ADDRESS(port) _MIPI_PORT(port, _MIPIA_DATA_ADDRESS, \
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 2c0ea6f..892b518 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -726,6 +726,21 @@ static void set_dsi_timings(struct drm_encoder *encoder,
hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
for_each_dsi_port(port, intel_dsi->ports) {
+ if (IS_BROXTON(dev)) {
+ /*
+ * Program hdisplay and vdisplay on MIPI transcoder.
+ * This is different from calculated hactive and
+ * vactive, as they are calculated per channel basis,
+ * whereas these values should be based on resolution.
+ */
+ I915_WRITE(BXT_MIPI_TRANS_HACTIVE(port),
+ mode->hdisplay);
+ I915_WRITE(BXT_MIPI_TRANS_VACTIVE(port),
+ mode->vdisplay);
+ I915_WRITE(BXT_MIPI_TRANS_VTOTAL(port),
+ mode->vtotal);
+ }
+
I915_WRITE(MIPI_HACTIVE_AREA_COUNT(port), hactive);
I915_WRITE(MIPI_HFP_COUNT(port), hfp);
@@ -766,16 +781,38 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
}
for_each_dsi_port(port, intel_dsi->ports) {
- /* escape clock divider, 20MHz, shared for A and C.
- * device ready must be off when doing this! txclkesc? */
- tmp = I915_READ(MIPI_CTRL(PORT_A));
- tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
- I915_WRITE(MIPI_CTRL(PORT_A), tmp | ESCAPE_CLOCK_DIVIDER_1);
-
- /* read request priority is per pipe */
- tmp = I915_READ(MIPI_CTRL(port));
- tmp &= ~READ_REQUEST_PRIORITY_MASK;
- I915_WRITE(MIPI_CTRL(port), tmp | READ_REQUEST_PRIORITY_HIGH);
+ if (IS_VALLEYVIEW(dev)) {
+ /*
+ * escape clock divider, 20MHz, shared for A and C.
+ * device ready must be off when doing this! txclkesc?
+ */
+ tmp = I915_READ(MIPI_CTRL(0));
+ tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
+ I915_WRITE(MIPI_CTRL(0), tmp |
+ ESCAPE_CLOCK_DIVIDER_1);
+
+ /* read request priority is per pipe */
+ tmp = I915_READ(MIPI_CTRL(port));
+ tmp &= ~READ_REQUEST_PRIORITY_MASK;
+ I915_WRITE(MIPI_CTRL(port), tmp |
+ READ_REQUEST_PRIORITY_HIGH);
+ } else if (IS_BROXTON(dev)) {
+ /*
+ * FIXME:
+ * BXT can connect any PIPE to any MIPI transcoder.
+ * Select the pipe based on the MIPI port read from
+ * VBT for now. Pick PIPE A for MIPI port A and C
+ * for port C.
+ */
+ tmp = I915_READ(MIPI_CTRL(port));
+ tmp &= ~BXT_PIPE_SELECT_MASK;
+ port ? (tmp |= BXT_PIPE_SELECT_C) :
+ (tmp |= BXT_PIPE_SELECT_A);
+ I915_WRITE(MIPI_CTRL(port), tmp);
+ } else {
+ DRM_ERROR("Invalid DSI device to prepare seq\n");
+ return;
+ }
/* XXX: why here, why like this? handling in irq handler?! */
I915_WRITE(MIPI_INTR_STAT(port), 0xffffffff);
@@ -852,6 +889,28 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
I915_WRITE(MIPI_INIT_COUNT(port),
txclkesc(intel_dsi->escape_clk_div, 100));
+ if (IS_BROXTON(dev)) {
+
+ /*
+ * XXX:
+ * PIPE2D says LOW_CONTENTION_RECOVERY_DISABLE is
+ * reqd for BXT, but bspec doesn't mention it.
+ * Lets follow PIPE2D for now.
+ */
+ val |= LOW_CONTENTION_RECOVERY_DISABLE;
+
+ if (!intel_dsi->dual_link) {
+ /*
+ * BXT spec says write MIPI_INIT_COUNT for
+ * both the ports, even if only one is
+ * getting used. So write the other port
+ * if not in dual link mode.
+ */
+ I915_WRITE(MIPI_INIT_COUNT(port ==
+ PORT_A ? PORT_C : PORT_A),
+ intel_dsi->init_count);
+ }
+ }
/* recovery disables */
I915_WRITE(MIPI_EOT_DISABLE(port), tmp);
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (3 preceding siblings ...)
2015-05-22 16:05 ` [PATCH 04/12] drm/i915/bxt: DSI prepare changes " Uma Shankar
@ 2015-05-22 16:05 ` Uma Shankar
2015-05-25 10:13 ` Jani Nikula
2015-05-25 10:25 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 06/12] drm/i915/bxt: DSI enable for BXT Uma Shankar
` (6 subsequent siblings)
11 siblings, 2 replies; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
SKL and BXT qualifies the HAS_DDI() check, and hence haswell modeset
functions are re-used for modeset sequence. But DDI interface doesn't
include support for DSI.
This patch adds:
1. cases for DSI encoder, in those modeset functions and allows a CRTC modeset
2. Adds call to pre_pll enabled from CRTC modeset function. Nothing needs to be
done as such in CRTC for DSI encoder, as PLL, clock and and transcoder programming
will be taken care in encoder's pre_enable and pre_pll_enable function.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_display.c | 6 +++-
drivers/gpu/drm/i915/intel_opregion.c | 1 +
4 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 840f08f..6874121 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2452,6 +2452,7 @@ struct drm_i915_cmd_table {
INTEL_INFO(dev)->gen >= 9)
#define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
+#define has_encoder_ddi(type) ((type) == (INTEL_OUTPUT_DSI) ? 0 : 1)
#define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d602db2..2aef8b5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -395,6 +395,12 @@ void intel_prepare_ddi(struct drm_device *dev)
if (visited[port])
continue;
+ if (intel_dig_port->base.type ==
+ INTEL_OUTPUT_DSI) {
+ visited[intel_dig_port->port] = true;
+ continue;
+ }
+
supports_hdmi = intel_dig_port &&
intel_dig_port_supports_hdmi(intel_dig_port);
@@ -1568,10 +1574,21 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
enum pipe pipe = intel_crtc->pipe;
enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
- enum port port = intel_ddi_get_encoder_port(intel_encoder);
+ enum port port;
int type = intel_encoder->type;
uint32_t temp;
+ /*
+ * Fixme: BXT has DDI, so tries to follow a DDI modeset function,
+ * but DDI interface doesn't support DSI yet, so don't do anything
+ * for DSI encoders
+ */
+ if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
+ DRM_DEBUG_DRIVER("Not setting transcoder for DSI\n");
+ return;
+ }
+
+ port = intel_ddi_get_encoder_port(intel_encoder);
/* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
temp = TRANS_DDI_FUNC_ENABLE;
temp |= TRANS_DDI_SELECT_PORT(port);
@@ -1779,11 +1796,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
{
struct drm_crtc *crtc = &intel_crtc->base;
- struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
- enum port port = intel_ddi_get_encoder_port(intel_encoder);
+ enum port port;
enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
+ int type = intel_encoder->type;
+ if (!(HAS_DDI(dev) && has_encoder_ddi(type)))
+ return;
+
+ port = intel_ddi_get_encoder_port(intel_encoder);
if (cpu_transcoder != TRANSCODER_EDP)
I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
TRANS_CLK_SEL_PORT(port));
@@ -1866,10 +1889,16 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
- enum port port = intel_ddi_get_encoder_port(intel_encoder);
+ enum port port;
int type = intel_encoder->type;
int hdmi_level;
+ if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
+ DRM_ERROR("DDI func getting called for MIPI?\n");
+ return;
+ }
+
+ port = intel_ddi_get_encoder_port(intel_encoder);
if (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
intel_edp_panel_on(intel_dp);
@@ -1942,11 +1971,17 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
struct drm_encoder *encoder = &intel_encoder->base;
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- enum port port = intel_ddi_get_encoder_port(intel_encoder);
+ enum port port;
int type = intel_encoder->type;
uint32_t val;
bool wait = false;
+ if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
+ DRM_ERROR("DDI func got called for DSI?\n");
+ return;
+ }
+
+ port = intel_ddi_get_encoder_port(intel_encoder);
val = I915_READ(DDI_BUF_CTL(port));
if (val & DDI_BUF_CTL_ENABLE) {
val &= ~DDI_BUF_CTL_ENABLE;
@@ -1983,9 +2018,15 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- enum port port = intel_ddi_get_encoder_port(intel_encoder);
+ enum port port;
int type = intel_encoder->type;
+ if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
+ DRM_ERROR("DDI func getting called for DSI?\n");
+ return;
+ }
+
+ port = intel_ddi_get_encoder_port(intel_encoder);
if (type == INTEL_OUTPUT_HDMI) {
struct intel_digital_port *intel_dig_port =
enc_to_dig_port(encoder);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c30bfd4..c1fac21 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4987,6 +4987,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
+ bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
WARN_ON(!crtc->state->enable);
@@ -5052,13 +5053,16 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
if (intel_crtc->config->has_pch_encoder)
lpt_pch_enable(crtc);
- if (intel_crtc->config->dp_encoder_is_mst)
+ if (intel_crtc->config->dp_encoder_is_mst && !is_dsi)
intel_ddi_set_vc_payload_alloc(crtc, true);
assert_vblank_disabled(crtc);
drm_crtc_vblank_on(crtc);
for_each_encoder_on_crtc(dev, crtc, encoder) {
+ if (encoder->pre_pll_enable)
+ encoder->pre_pll_enable(encoder);
+
encoder->enable(encoder);
intel_opregion_notify_encoder(encoder, true);
}
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 71e87ab..4b025ee 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -356,6 +356,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
break;
case INTEL_OUTPUT_EDP:
+ case INTEL_OUTPUT_DSI:
type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
break;
default:
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 06/12] drm/i915/bxt: DSI enable for BXT
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (4 preceding siblings ...)
2015-05-22 16:05 ` [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset Uma Shankar
@ 2015-05-22 16:05 ` Uma Shankar
2015-05-25 16:39 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks Uma Shankar
` (5 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:05 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
This patch contains following changes:
1. MIPI device ready changes to support dsi_pre_enable. Changes
are specific to BXT device ready sequence. Added check for
ULPS mode(No effects on VLV).
2. Changes in dsi_enable to pick BXT port control register.
3. Changes in dsi_pre_enable to restrict DPIO programming for VLV
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 7 ++
drivers/gpu/drm/i915/intel_dsi.c | 185 ++++++++++++++++++++++++++++----------
2 files changed, 144 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 57c3a48..5eeb2b7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7408,6 +7408,13 @@ enum skl_disp_power_wells {
#define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
#define _MIPIC_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
#define MIPI_PORT_CTRL(port) _MIPI_PORT(port, _MIPIA_PORT_CTRL, _MIPIC_PORT_CTRL)
+
+ /* BXT port control */
+#define _BXT_MIPIA_PORT_CTRL 0x6B0C0
+#define _BXT_MIPIC_PORT_CTRL 0x6B8C0
+#define BXT_MIPI_PORT_CTRL(tc) _TRANSCODER(tc, _BXT_MIPIA_PORT_CTRL, \
+ _BXT_MIPIC_PORT_CTRL)
+
#define DPI_ENABLE (1 << 31) /* A + C */
#define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
#define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 892b518..3a1bb04 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -286,6 +286,101 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
return true;
}
+static void bxt_dsi_device_ready(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ enum port port;
+ u32 val;
+
+ DRM_DEBUG_KMS("\n");
+
+ /* Exit Low power state in 4 steps*/
+ for_each_dsi_port(port, intel_dsi->ports) {
+
+ /* 1. Enable MIPI PHY transparent latch */
+ val = I915_READ(BXT_MIPI_PORT_CTRL(port));
+ I915_WRITE(BXT_MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
+ usleep_range(2000, 2500);
+
+ /* 2. Enter ULPS */
+ val = I915_READ(MIPI_DEVICE_READY(port));
+ val &= ~ULPS_STATE_MASK;
+ val |= (ULPS_STATE_ENTER | DEVICE_READY);
+ I915_WRITE(MIPI_DEVICE_READY(port), val);
+ usleep_range(2, 3);
+
+ /* 3. Exit ULPS */
+ val = I915_READ(MIPI_DEVICE_READY(port));
+ val &= ~ULPS_STATE_MASK;
+ val |= (ULPS_STATE_EXIT | DEVICE_READY);
+ I915_WRITE(MIPI_DEVICE_READY(port), val);
+ usleep_range(1000, 1500);
+
+ /* Clear ULPS and set device ready */
+ val = I915_READ(MIPI_DEVICE_READY(port));
+ val &= ~ULPS_STATE_MASK;
+ val |= DEVICE_READY;
+ I915_WRITE(MIPI_DEVICE_READY(port), val);
+ }
+}
+
+static void vlv_dsi_device_ready(struct intel_encoder *encoder)
+{
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+ int pipe = intel_crtc->pipe;
+ u32 val;
+ int count = 1;
+ u32 reg = 0;
+ bool afe_latchout = true;
+
+ DRM_DEBUG_KMS("\n");
+
+ pipe = intel_crtc->pipe;
+ reg = MIPI_PORT_CTRL(pipe);
+
+ /* Ceck LP state */
+ val = I915_READ(reg);
+ afe_latchout = (val & AFE_LATCHOUT);
+
+ /* Enter low power state, if not already in */
+ if (afe_latchout) {
+ /* Enter ULPS, wait for 2.5 ms */
+ reg = MIPI_DEVICE_READY(pipe);
+ I915_WRITE(reg, val | ULPS_STATE_ENTER);
+ usleep_range(2000, 2500);
+ }
+
+ /* Enable transparent latch */
+ I915_WRITE(reg, val | LP_OUTPUT_HOLD);
+
+ if (intel_dsi->dual_link) {
+ count = 2;
+ pipe = PIPE_A;
+ }
+
+ do {
+ I915_WRITE(reg, val | ULPS_STATE_EXIT);
+ usleep_range(2000, 2500);
+ /* Clear ULPS state */
+ val = I915_READ(MIPI_DEVICE_READY(pipe)) & ~ULPS_STATE_MASK;
+ I915_WRITE(MIPI_DEVICE_READY(pipe), val);
+ usleep_range(2000, 2500);
+
+ /* Set device ready */
+ val |= DEVICE_READY;
+ I915_WRITE(MIPI_DEVICE_READY(pipe), val);
+ usleep_range(2000, 2500);
+
+ /* For Port C for dual link */
+ if (intel_dsi->dual_link)
+ pipe = PIPE_B;
+ } while (--count > 0);
+}
+
static void intel_dsi_port_enable(struct intel_encoder *encoder)
{
struct drm_device *dev = encoder->base.dev;
@@ -294,6 +389,7 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
enum port port;
u32 temp;
+ u32 port_ctrl;
if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
temp = I915_READ(VLV_CHICKEN_3);
@@ -304,7 +400,10 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
}
for_each_dsi_port(port, intel_dsi->ports) {
- temp = I915_READ(MIPI_PORT_CTRL(port));
+ port_ctrl = IS_BROXTON(dev) ? BXT_MIPI_PORT_CTRL(port) :
+ MIPI_PORT_CTRL(port);
+ temp = I915_READ(port_ctrl);
+
temp &= ~LANE_CONFIGURATION_MASK;
temp &= ~DUAL_LINK_MODE_MASK;
@@ -316,8 +415,8 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
LANE_CONFIGURATION_DUAL_LINK_A;
}
/* assert ip_tg_enable signal */
- I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
- POSTING_READ(MIPI_PORT_CTRL(port));
+ I915_WRITE(port_ctrl, temp | DPI_ENABLE);
+ POSTING_READ(port_ctrl);
}
}
@@ -339,41 +438,15 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
static void intel_dsi_device_ready(struct intel_encoder *encoder)
{
- struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
- struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
- enum port port;
- u32 val;
-
- DRM_DEBUG_KMS("\n");
-
- mutex_lock(&dev_priv->dpio_lock);
- /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
- * needed everytime after power gate */
- vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
- mutex_unlock(&dev_priv->dpio_lock);
-
- /* bandgap reset is needed after everytime we do power gate */
- band_gap_reset(dev_priv);
-
- for_each_dsi_port(port, intel_dsi->ports) {
-
- I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER);
- usleep_range(2500, 3000);
-
- /* Enable MIPI PHY transparent latch
- * Common bit for both MIPI Port A & MIPI Port C
- * No similar bit in MIPI Port C reg
- */
- val = I915_READ(MIPI_PORT_CTRL(PORT_A));
- I915_WRITE(MIPI_PORT_CTRL(PORT_A), val | LP_OUTPUT_HOLD);
- usleep_range(1000, 1500);
+ struct drm_device *dev = encoder->base.dev;
- I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
- usleep_range(2500, 3000);
+ if (IS_VALLEYVIEW(dev))
+ vlv_dsi_device_ready(encoder);
+ else if (IS_BROXTON(dev))
+ bxt_dsi_device_ready(encoder);
+ else
+ DRM_ERROR("Invalid DSI device to device ready\n");
- I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
- usleep_range(2500, 3000);
- }
}
static void intel_dsi_enable(struct intel_encoder *encoder)
@@ -415,19 +488,22 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
DRM_DEBUG_KMS("\n");
- /* Disable DPOunit clock gating, can stall pipe
- * and we need DPLL REFA always enabled */
- tmp = I915_READ(DPLL(pipe));
- tmp |= DPLL_REFA_CLK_ENABLE_VLV;
- I915_WRITE(DPLL(pipe), tmp);
+ if (IS_VALLEYVIEW(dev)) {
+ /* Disable DPOunit clock gating, can stall pipe
+ * and we need DPLL REFA always enabled
+ */
+ tmp = I915_READ(DPLL(pipe));
+ tmp |= DPLL_REFA_CLK_ENABLE_VLV;
+ I915_WRITE(DPLL(pipe), tmp);
- /* update the hw state for DPLL */
- intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
- DPLL_REFA_CLK_ENABLE_VLV;
+ /* update the hw state for DPLL */
+ intel_crtc->config->dpll_hw_state.dpll =
+ DPLL_INTEGRATED_CLOCK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
- tmp = I915_READ(DSPCLK_GATE_D);
- tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
- I915_WRITE(DSPCLK_GATE_D, tmp);
+ tmp = I915_READ(DSPCLK_GATE_D);
+ tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
+ I915_WRITE(DSPCLK_GATE_D, tmp);
+ }
/* put device in ready state */
intel_dsi_device_ready(encoder);
@@ -959,11 +1035,24 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
{
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
DRM_DEBUG_KMS("\n");
+ if (IS_VALLEYVIEW(dev)) {
+ mutex_lock(&dev_priv->dpio_lock);
+ /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
+ * needed everytime after power gate */
+ vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
+ mutex_unlock(&dev_priv->dpio_lock);
+
+ /* bandgap reset is needed after everytime we do power gate */
+ band_gap_reset(dev_priv);
+ }
+
intel_dsi_prepare(encoder);
intel_enable_dsi_pll(encoder);
-
}
static enum drm_connector_status
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (5 preceding siblings ...)
2015-05-22 16:05 ` [PATCH 06/12] drm/i915/bxt: DSI enable for BXT Uma Shankar
@ 2015-05-22 16:06 ` Uma Shankar
2015-05-25 16:52 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 08/12] drm/i915/bxt: DSI disable and post-disable Uma Shankar
` (4 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:06 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
BXT DSI clocks are different than previous platforms. So adding a
new function to program following clocks and dividers:
1. Program variable divider to generate input to Tx clock divider
(Output value must be < 39.5Mhz)
2. Select divide by 2 option to get < 20Mhz for Tx clock
3. Program 8by3 divider to generate Rx clock
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 51 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_dsi.c | 3 ++
drivers/gpu/drm/i915/intel_dsi.h | 1 +
drivers/gpu/drm/i915/intel_dsi_pll.c | 35 +++++++++++++++++++++++
4 files changed, 90 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5eeb2b7..f96f049 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7387,6 +7387,57 @@ enum skl_disp_power_wells {
#define GEN9_FUSE_PG1_ENABLED (1 << 26)
#define GEN9_FUSE_PG0_ENABLED (1 << 27)
+/* BXT MIPI clock controls */
+#define BXT_MAX_VAR_OUTPUT_KHZ 39500
+#define BXT_MIPI_CLOCK_CTL 0x46090
+#define BXT_MIPI_DIV_SHIFT 16
+
+/* Var clock divider to generate TX source. Result must be < 39.5 M */
+#define BXT_MIPI1_ESCLK_VAR_DIV_MASK (0x3F << 26)
+/* BXT MIPI clock controls */
+#define BXT_MAX_VAR_OUTPUT_KHZ 39500
+#define BXT_MIPI_CLOCK_CTL 0x46090
+#define BXT_MIPI_DIV_SHIFT 16
+
+/* Var clock divider to generate TX source. Result must be < 39.5 M */
+#define BXT_MIPI1_ESCLK_VAR_DIV_MASK (0x3F << 26)
+#define BXT_MIPI2_ESCLK_VAR_DIV_MASK (0x3F << 10)
+#define BXT_MIPI_ESCLK_VAR_DIV_MASK(check) \
+ (0x3F << ((!check) * BXT_MIPI_DIV_SHIFT + 10))
+#define BXT_MIPI_ESCLK_VAR_DIV(check, val) \
+ (val << ((!check) * BXT_MIPI_DIV_SHIFT + 10))
+
+/* TX control divider to select actual TX clock output from (8x/var) */
+#define BXT_MIPI1_TX_ESCLK_FIXDIV_MASK (3 << 21)
+#define BXT_MIPI2_TX_ESCLK_FIXDIV_MASK (3 << 5)
+#define BXT_MIPI_TX_ESCLK_FIXDIV_MASK(check) \
+ (3 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
+
+#define BXT_MIPI_TX_ESCLK_8XDIV_BY2(check) \
+ (0x0 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
+#define BXT_MIPI_TX_ESCLK_8XDIV_BY4(check) \
+ (0x1 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
+#define BXT_MIPI_TX_ESCLK_8XDIV_BY8(check) \
+ (0x2 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
+
+/* RX control divider to select actual RX clock output from 8x*/
+#define BXT_MIPI1_RX_ESCLK_FIXDIV_MASK (3 << 19)
+#define BXT_MIPI2_RX_ESCLK_FIXDIV_MASK (3 << 3)
+#define BXT_MIPI_RX_ESCLK_FIXDIV_MASK(check) \
+ (3 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
+#define BXT_MIPI_RX_ESCLK_8X_BY2(check) \
+ (1 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
+#define BXT_MIPI_RX_ESCLK_8X_BY3(check) \
+ (2 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
+#define BXT_MIPI_RX_ESCLK_8X_BY4(check) \
+ (3 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
+
+/* BXT: Always prog DPHY dividers to 00 */
+#define BXT_MIPI_1_DPHY_DIVIDER_MASK (3 << 16)
+#define BXT_MIPI_2_DPHY_DIVIDER_MASK (3 << 0)
+#define BXT_MIPI_DPHY_DIVIDER_MASK(check) \
+ (3 << ((!check) * BXT_MIPI_DIV_SHIFT))
+
/* BXT MIPI mode configure */
#define _BXT_MIPIA_TRANS_HACTIVE 0x6B0F8
#define _BXT_MIPIC_TRANS_HACTIVE 0x6B8F8
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 3a1bb04..729faf6 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -503,6 +503,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
tmp = I915_READ(DSPCLK_GATE_D);
tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
I915_WRITE(DSPCLK_GATE_D, tmp);
+ } else if (IS_BROXTON(dev)) {
+ /* Program Tx Rx and Dphy clocks */
+ bxt_dsi_program_clocks(dev, intel_dsi->ports);
}
/* put device in ready state */
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 759983e..af5a09f 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -124,6 +124,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
+extern void bxt_dsi_program_clocks(struct drm_device *dev, int pipe);
struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 0cbcf32..9a8a35d 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -369,6 +369,41 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
return pclk;
}
+/* Program BXT Mipi clocks and dividers */
+void bxt_dsi_program_clocks(struct drm_device *dev, int pipe)
+{
+ u32 tmp = 0;
+ u32 divider = 0;
+ u32 dsi_rate = 0;
+ u32 pll_ratio = 0;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ /* Clear old configurations */
+ tmp = I915_READ(BXT_MIPI_CLOCK_CTL);
+ tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(pipe));
+ tmp &= ~(BXT_MIPI_RX_ESCLK_FIXDIV_MASK(pipe));
+ tmp &= ~(BXT_MIPI_ESCLK_VAR_DIV_MASK(pipe));
+ tmp &= ~(BXT_MIPI_DPHY_DIVIDER_MASK(pipe));
+
+ /* Get the current DSI rate(actual) */
+ pll_ratio = I915_READ(BXT_DSI_PLL_CTL) &
+ BXT_DSI_MASK_PLL_RATIO;
+ dsi_rate = (BXT_REF_CLOCK_KHZ * pll_ratio) / 2;
+
+ /* Max possible output of clock is 39.5 MHz, program value -1 */
+ divider = (dsi_rate / BXT_MAX_VAR_OUTPUT_KHZ) - 1;
+ tmp |= BXT_MIPI_ESCLK_VAR_DIV(pipe, divider);
+
+ /* Tx escape clock should be >=20MHz, so select divide by 2 */
+ tmp |= BXT_MIPI_TX_ESCLK_8XDIV_BY2(pipe);
+
+ /* Rx escape clock, select fix divide by 3 clock */
+ tmp |= BXT_MIPI_RX_ESCLK_8X_BY3(pipe);
+
+ /* Do the honors */
+ I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
+}
+
static bool bxt_configure_dsi_pll(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 08/12] drm/i915/bxt: DSI disable and post-disable
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (6 preceding siblings ...)
2015-05-22 16:06 ` [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks Uma Shankar
@ 2015-05-22 16:06 ` Uma Shankar
2015-05-25 16:44 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 09/12] drm/i915/bxt: get_hw_state for BXT Uma Shankar
` (3 subsequent siblings)
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:06 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
This patch contains changes to support DSI disble sequence in BXT.
The changes are:
1. BXT specific changes in clear_device_ready function.
2. BXT specific changes in DSI disable and post-disable functions.
3. Add a new function to reset BXT Dphy clock and dividers
(bxt_dsi_reset_clocks).
4. Moved some part of the vlv clock reset code, in a new function
(vlv_dsi_reset_clocks) maintaining the exact same sequence.
5. Wrapper function to call corresponding reset clock function.
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_dsi.c | 39 ++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_dsi.h | 2 ++
drivers/gpu/drm/i915/intel_dsi_pll.c | 41 ++++++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 729faf6..e4b96bc 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -427,12 +427,16 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
enum port port;
u32 temp;
+ u32 port_ctrl;
for_each_dsi_port(port, intel_dsi->ports) {
/* de-assert ip_tg_enable signal */
- temp = I915_READ(MIPI_PORT_CTRL(port));
- I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
- POSTING_READ(MIPI_PORT_CTRL(port));
+ port_ctrl = IS_BROXTON(dev) ?
+ BXT_MIPI_PORT_CTRL(port) :
+ MIPI_PORT_CTRL(port);
+ temp = I915_READ(port_ctrl);
+ I915_WRITE(port_ctrl, temp & ~DPI_ENABLE);
+ POSTING_READ(port_ctrl);
}
}
@@ -570,12 +574,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
/* Panel commands can be sent when clock is in LP11 */
I915_WRITE(MIPI_DEVICE_READY(port), 0x0);
- temp = I915_READ(MIPI_CTRL(port));
- temp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
- I915_WRITE(MIPI_CTRL(port), temp |
- intel_dsi->escape_clk_div <<
- ESCAPE_CLOCK_DIVIDER_SHIFT);
-
+ intel_dsi_reset_clocks(encoder, port);
I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);
temp = I915_READ(MIPI_DSI_FUNC_PRG(port));
@@ -594,12 +593,15 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
{
+ struct drm_device *dev = encoder->base.dev;
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
enum port port;
u32 val;
+ u32 port_ctrl;
DRM_DEBUG_KMS("\n");
+
for_each_dsi_port(port, intel_dsi->ports) {
I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY |
@@ -614,18 +616,27 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
ULPS_STATE_ENTER);
usleep_range(2000, 2500);
+ if (IS_BROXTON(dev))
+ port_ctrl = BXT_MIPI_PORT_CTRL(port);
+ else if (IS_VALLEYVIEW(dev))
+ port_ctrl = MIPI_PORT_CTRL(PORT_A);
+ else {
+ DRM_ERROR("Invalid DSI device to clear device ready\n");
+ return;
+ }
+
/* Wait till Clock lanes are in LP-00 state for MIPI Port A
* only. MIPI Port C has no similar bit for checking
*/
- if (wait_for(((I915_READ(MIPI_PORT_CTRL(PORT_A)) & AFE_LATCHOUT)
+ if (wait_for(((I915_READ(port_ctrl) & AFE_LATCHOUT)
== 0x00000), 30))
DRM_ERROR("DSI LP not going Low\n");
/* Disable MIPI PHY transparent latch
* Common bit for both MIPI Port A & MIPI Port C
*/
- val = I915_READ(MIPI_PORT_CTRL(PORT_A));
- I915_WRITE(MIPI_PORT_CTRL(PORT_A), val & ~LP_OUTPUT_HOLD);
+ val = I915_READ(port_ctrl);
+ I915_WRITE(port_ctrl, val & ~LP_OUTPUT_HOLD);
usleep_range(1000, 1500);
I915_WRITE(MIPI_DEVICE_READY(port), 0x00);
@@ -637,14 +648,14 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
static void intel_dsi_post_disable(struct intel_encoder *encoder)
{
- struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
u32 val;
DRM_DEBUG_KMS("\n");
intel_dsi_disable(encoder);
-
intel_dsi_clear_device_ready(encoder);
val = I915_READ(DSPCLK_GATE_D);
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index af5a09f..8bc8d94 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -125,6 +125,8 @@ extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
extern void bxt_dsi_program_clocks(struct drm_device *dev, int pipe);
+extern void intel_dsi_reset_clocks(struct intel_encoder *encoder,
+ enum port port);
struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 9a8a35d..49330b0 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -369,6 +369,19 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
return pclk;
}
+void vlv_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
+{
+ u32 temp;
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+
+ temp = I915_READ(MIPI_CTRL(port));
+ temp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
+ I915_WRITE(MIPI_CTRL(port), temp |
+ intel_dsi->escape_clk_div <<
+ ESCAPE_CLOCK_DIVIDER_SHIFT);
+}
+
/* Program BXT Mipi clocks and dividers */
void bxt_dsi_program_clocks(struct drm_device *dev, int pipe)
{
@@ -515,3 +528,31 @@ void intel_disable_dsi_pll(struct intel_encoder *encoder)
else
DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
}
+
+void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
+{
+ u32 tmp;
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ /* Clear old configurations */
+ tmp = I915_READ(BXT_MIPI_CLOCK_CTL);
+ tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(port));
+ tmp &= ~(BXT_MIPI_RX_ESCLK_FIXDIV_MASK(port));
+ tmp &= ~(BXT_MIPI_ESCLK_VAR_DIV_MASK(port));
+ tmp &= ~(BXT_MIPI_DPHY_DIVIDER_MASK(port));
+ I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
+ I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);
+}
+
+void intel_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
+{
+ struct drm_device *dev = encoder->base.dev;
+
+ if (IS_BROXTON(dev))
+ bxt_dsi_reset_clocks(encoder, port);
+ else if (IS_VALLEYVIEW(dev))
+ vlv_dsi_reset_clocks(encoder, port);
+ else
+ DRM_ERROR("Invalid DSI device to reset clocks\n");
+}
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 09/12] drm/i915/bxt: get_hw_state for BXT
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (7 preceding siblings ...)
2015-05-22 16:06 ` [PATCH 08/12] drm/i915/bxt: DSI disable and post-disable Uma Shankar
@ 2015-05-22 16:06 ` Uma Shankar
2015-05-22 16:06 ` [PATCH 10/12] drm/i915/bxt: get DSI pixelclock Uma Shankar
` (2 subsequent siblings)
11 siblings, 0 replies; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:06 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
Pick appropriate port control register (BXT or VLV), based on device.
Get the current hw state wrt Mipi port.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_dsi.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index e4b96bc..2663cdd 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -675,7 +675,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
struct drm_device *dev = encoder->base.dev;
enum intel_display_power_domain power_domain;
- u32 dpi_enabled, func;
+ u32 dpi_enabled, func, ctrl_reg;
enum port port;
DRM_DEBUG_KMS("\n");
@@ -687,8 +687,9 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
/* XXX: this only works for one DSI output */
for_each_dsi_port(port, intel_dsi->ports) {
func = I915_READ(MIPI_DSI_FUNC_PRG(port));
- dpi_enabled = I915_READ(MIPI_PORT_CTRL(port)) &
- DPI_ENABLE;
+ ctrl_reg = IS_BROXTON(dev) ? BXT_MIPI_PORT_CTRL(port) :
+ MIPI_PORT_CTRL(port);
+ dpi_enabled = I915_READ(ctrl_reg) & DPI_ENABLE;
/* Due to some hardware limitations on BYT, MIPI Port C DPI
* Enable bit does not get set. To check whether DSI Port C
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 10/12] drm/i915/bxt: get DSI pixelclock
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (8 preceding siblings ...)
2015-05-22 16:06 ` [PATCH 09/12] drm/i915/bxt: get_hw_state for BXT Uma Shankar
@ 2015-05-22 16:06 ` Uma Shankar
2015-05-25 16:54 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes Uma Shankar
2015-05-22 16:06 ` [PATCH 12/12] drm/i915/bxt: Remove DSP CLK_GATE programming for BXT Uma Shankar
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:06 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Shashank Sharma <shashank.sharma@intel.com>
BXT's DSI PLL is different from that of VLV. So this patch
adds a new function to get the current DSI pixel clock based
on the PLL divider ratio and lane count.
This function is required for intel_dsi_get_config() function.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_dsi.c | 10 ++++++++--
drivers/gpu/drm/i915/intel_dsi.h | 1 +
drivers/gpu/drm/i915/intel_dsi_pll.c | 35 ++++++++++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 2663cdd..5fbcebe 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -714,7 +714,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
static void intel_dsi_get_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config)
{
- u32 pclk;
+ u32 pclk = 0;
DRM_DEBUG_KMS("\n");
/*
@@ -723,7 +723,13 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
*/
pipe_config->dpll_hw_state.dpll_md = 0;
- pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
+ if (IS_BROXTON(encoder->base.dev))
+ pclk = bxt_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
+ else if (IS_VALLEYVIEW(encoder->base.dev))
+ pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
+ else
+ DRM_ERROR("Invalid DSI device to get config\n");
+
if (!pclk)
return;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 8bc8d94..01e08e7 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -124,6 +124,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
+extern u32 bxt_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
extern void bxt_dsi_program_clocks(struct drm_device *dev, int pipe);
extern void intel_dsi_reset_clocks(struct intel_encoder *encoder,
enum port port);
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index 49330b0..073bd1f 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -369,6 +369,41 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
return pclk;
}
+u32 bxt_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
+{
+ u32 pclk;
+ u32 dsi_clk;
+ u32 dsi_ratio;
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+
+ /* Divide by zero */
+ if (!pipe_bpp) {
+ DRM_ERROR("Invalid BPP(0)\n");
+ return 0;
+ }
+
+ dsi_ratio = I915_READ(BXT_DSI_PLL_CTL) &
+ BXT_DSI_MASK_PLL_RATIO;
+
+ /* Invalid DSI ratio ? */
+ if (dsi_ratio < BXT_DSI_PLL_RATIO_MIN ||
+ dsi_ratio > BXT_DSI_PLL_RATIO_MAX) {
+ DRM_ERROR("Invalid DSI pll ratio(%u) programmed\n", dsi_ratio);
+ return 0;
+ }
+
+ dsi_clk = (dsi_ratio * BXT_REF_CLOCK_KHZ) / 2;
+
+ /* pixel_format and pipe_bpp should agree */
+ assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
+
+ pclk = DIV_ROUND_CLOSEST(dsi_clk * intel_dsi->lane_count, pipe_bpp);
+
+ DRM_DEBUG_DRIVER("Calculated pclk=%u\n", pclk);
+ return pclk;
+}
+
void vlv_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
{
u32 temp;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (9 preceding siblings ...)
2015-05-22 16:06 ` [PATCH 10/12] drm/i915/bxt: get DSI pixelclock Uma Shankar
@ 2015-05-22 16:06 ` Uma Shankar
2015-05-25 10:03 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 12/12] drm/i915/bxt: Remove DSP CLK_GATE programming for BXT Uma Shankar
11 siblings, 1 reply; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:06 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
From: Sunil Kamath <sunil.kamath@intel.com>
Latest VBT mentions which set of registers will be used for BLC,
as controller number field. Making use of this field in BXT
BLC implementation. Also, the registers are used in case control
pin indicates display DDI. Adding a check for this.
According to Bspec, BLC_PWM_*_2 uses the display utility pin for output.
To use backlight 2, enable the utility pin with mode = PWM
v2: Jani's review comments
addressed
- Add a prefix _ to BXT BLC registers definitions.
- Add "bxt only" comment for u8 controller
- Remove control_pin check for DDI controller
- Check for valid controller values
- Set pipe bits in UTIL_PIN_CTL
- Enable/Disable UTIL_PIN_CTL in enable/disable_backlight()
- If BLC 2 is used, read active_low_pwm from UTIL_PIN polarity
Satheesh's review comment addressed
- If UTIL PIN is already enabled, BIOS would have programmed it. No
need to disable and enable again.
v3: Jani's review comments
- add UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK
- Disable UTIL_PIN if controller 1 is used
- Mask out UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK before enabling
UTIL_PIN
- check valid controller value in intel_bios.c
- add backlight.util_pin_active_low
- disable util pin before enabling
v4: Change for BXT-PO branch:
Stubbed unwanted definition which was existing before
because of DC6 patch.
UTIL_PIN_MODE_PWM (0x1b << 24)
Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 28 +++++++---
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_panel.c | 100 ++++++++++++++++++++++++++++++------
3 files changed, 106 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f96f049..5e3958f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3509,17 +3509,29 @@ enum skl_disp_power_wells {
#define UTIL_PIN_CTL 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)
+
/* BXT backlight register definition. */
-#define BXT_BLC_PWM_CTL1 0xC8250
+#define _BXT_BLC_PWM_CTL1 0xC8250
#define BXT_BLC_PWM_ENABLE (1 << 31)
#define BXT_BLC_PWM_POLARITY (1 << 29)
-#define BXT_BLC_PWM_FREQ1 0xC8254
-#define BXT_BLC_PWM_DUTY1 0xC8258
-
-#define BXT_BLC_PWM_CTL2 0xC8350
-#define BXT_BLC_PWM_FREQ2 0xC8354
-#define BXT_BLC_PWM_DUTY2 0xC8358
-
+#define _BXT_BLC_PWM_FREQ1 0xC8254
+#define _BXT_BLC_PWM_DUTY1 0xC8258
+
+#define _BXT_BLC_PWM_CTL2 0xC8350
+#define _BXT_BLC_PWM_FREQ2 0xC8354
+#define _BXT_BLC_PWM_DUTY2 0xC8358
+
+#define BXT_BLC_PWM_CTL(controller) _PIPE(controller, \
+ _BXT_BLC_PWM_CTL1, _BXT_BLC_PWM_CTL2)
+#define BXT_BLC_PWM_FREQ(controller) _PIPE(controller, \
+ _BXT_BLC_PWM_FREQ1, _BXT_BLC_PWM_FREQ2)
+#define BXT_BLC_PWM_DUTY(controller) _PIPE(controller, \
+ _BXT_BLC_PWM_DUTY1, _BXT_BLC_PWM_DUTY2)
#define PCH_GTC_CTL 0xe7000
#define PCH_GTC_ENABLE (1 << 31)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47bc729..ee9eb2b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -182,7 +182,9 @@ struct intel_panel {
bool enabled;
bool combination_mode; /* gen 2/4 only */
bool active_low_pwm;
+ bool util_pin_active_low; /* bxt+ */
struct backlight_device *device;
+ u8 controller; /* bxt+ only */
} backlight;
void (*backlight_power)(struct intel_connector *, bool enable);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 7d83527..23847f2 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -32,7 +32,10 @@
#include <linux/kernel.h>
#include <linux/moduleparam.h>
+#include <drm/drm_panel.h>
#include "intel_drv.h"
+#include "intel_dsi.h"
+#include "i915_drv.h"
void
intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
@@ -539,9 +542,10 @@ static u32 vlv_get_backlight(struct intel_connector *connector)
static u32 bxt_get_backlight(struct intel_connector *connector)
{
struct drm_device *dev = connector->base.dev;
+ struct intel_panel *panel = &connector->panel;
struct drm_i915_private *dev_priv = dev->dev_private;
- return I915_READ(BXT_BLC_PWM_DUTY1);
+ return I915_READ(BXT_BLC_PWM_DUTY(panel->backlight.controller));
}
static u32 intel_panel_get_backlight(struct intel_connector *connector)
@@ -628,8 +632,9 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level)
{
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_panel *panel = &connector->panel;
- I915_WRITE(BXT_BLC_PWM_DUTY1, level);
+ I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller), level);
}
static void
@@ -761,12 +766,20 @@ static void bxt_disable_backlight(struct intel_connector *connector)
{
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 tmp;
+ struct intel_panel *panel = &connector->panel;
+ u32 tmp, val;
intel_panel_actually_set_backlight(connector, 0);
- tmp = I915_READ(BXT_BLC_PWM_CTL1);
- I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE);
+ tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
+ I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
+ tmp & ~BXT_BLC_PWM_ENABLE);
+
+ if (panel->backlight.controller == 1) {
+ val = I915_READ(UTIL_PIN_CTL);
+ val &= ~UTIL_PIN_ENABLE;
+ I915_WRITE(UTIL_PIN_CTL, val);
+ }
}
void intel_panel_disable_backlight(struct intel_connector *connector)
@@ -980,16 +993,39 @@ static void bxt_enable_backlight(struct intel_connector *connector)
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_panel *panel = &connector->panel;
- u32 pwm_ctl;
+ enum pipe pipe = intel_get_pipe_from_connector(connector);
+ u32 pwm_ctl, val;
- pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
+ /* To use 2nd set of backlight registers, utility pin has to be
+ * enabled with PWM mode.
+ * The field should only be changed when the utility pin is disabled
+ */
+ if (panel->backlight.controller == 1) {
+ val = I915_READ(UTIL_PIN_CTL);
+ if (val & UTIL_PIN_ENABLE) {
+ DRM_DEBUG_KMS("util pin already enabled\n");
+ val &= ~UTIL_PIN_ENABLE;
+ I915_WRITE(UTIL_PIN_CTL, val);
+ }
+ /* mask out UTIL_PIN_PIPE and UTIL_PIN_MODE */
+ val &= ~(UTIL_PIN_PIPE_MASK | UTIL_PIN_MODE_MASK);
+ I915_WRITE(UTIL_PIN_CTL, val);
+ if (panel->backlight.util_pin_active_low)
+ val |= UTIL_PIN_POLARITY;
+ I915_WRITE(UTIL_PIN_CTL, val | UTIL_PIN_PIPE(pipe) |
+ UTIL_PIN_MODE_PWM | UTIL_PIN_ENABLE);
+ }
+
+ pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
if (pwm_ctl & BXT_BLC_PWM_ENABLE) {
DRM_DEBUG_KMS("backlight already enabled\n");
pwm_ctl &= ~BXT_BLC_PWM_ENABLE;
- I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
+ I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
+ pwm_ctl);
}
- I915_WRITE(BXT_BLC_PWM_FREQ1, panel->backlight.max);
+ I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
+ panel->backlight.max);
intel_panel_actually_set_backlight(connector, panel->backlight.level);
@@ -997,9 +1033,10 @@ static void bxt_enable_backlight(struct intel_connector *connector)
if (panel->backlight.active_low_pwm)
pwm_ctl |= BXT_BLC_PWM_POLARITY;
- I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
- POSTING_READ(BXT_BLC_PWM_CTL1);
- I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE);
+ I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), pwm_ctl);
+ POSTING_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
+ I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
+ pwm_ctl | BXT_BLC_PWM_ENABLE);
}
void intel_panel_enable_backlight(struct intel_connector *connector)
@@ -1008,6 +1045,9 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_panel *panel = &connector->panel;
enum pipe pipe = intel_get_pipe_from_connector(connector);
+ struct intel_dsi *intel_dsi = NULL;
+ struct drm_crtc *crtc = NULL;
+ struct intel_encoder *encoder = NULL;
if (!panel->backlight.present)
return;
@@ -1027,7 +1067,18 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
panel->backlight.device->props.max_brightness);
}
- dev_priv->display.enable_backlight(connector);
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ for_each_encoder_on_crtc(dev, crtc, encoder) {
+ if (encoder->type == INTEL_OUTPUT_DSI)
+ intel_dsi = enc_to_intel_dsi(&encoder->base);
+ }
+ }
+
+ if (IS_BROXTON(dev) && intel_dsi->panel->funcs->enable)
+ intel_dsi->panel->funcs->enable(intel_dsi->panel);
+ else
+ dev_priv->display.enable_backlight(connector);
+
panel->backlight.enabled = true;
if (panel->backlight.device)
panel->backlight.device->props.power = FB_BLANK_UNBLANK;
@@ -1362,10 +1413,27 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
struct intel_panel *panel = &connector->panel;
u32 pwm_ctl, val;
- pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
- panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
+ /* For BXT hard coding the Backlight controller to 0.
+ * ToDo : Read the controller value from VBT and generalize
+ */
+ panel->backlight.controller = 0;
+
+ pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
+
+ /* Keeping the check if controller 1 is to be programmed.
+ * This will come into affect once the VBT parsing
+ * is fixed for controller selection, and controller 1 is used
+ * for a prticular display configuration.
+ */
+ if (panel->backlight.controller == 1) {
+ val = I915_READ(UTIL_PIN_CTL);
+ panel->backlight.util_pin_active_low =
+ val & UTIL_PIN_POLARITY;
+ }
- panel->backlight.max = I915_READ(BXT_BLC_PWM_FREQ1);
+ panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
+ panel->backlight.max = I915_READ(
+ BXT_BLC_PWM_FREQ(panel->backlight.controller));
if (!panel->backlight.max)
return -ENODEV;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 12/12] drm/i915/bxt: Remove DSP CLK_GATE programming for BXT
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
` (10 preceding siblings ...)
2015-05-22 16:06 ` [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes Uma Shankar
@ 2015-05-22 16:06 ` Uma Shankar
11 siblings, 0 replies; 28+ messages in thread
From: Uma Shankar @ 2015-05-22 16:06 UTC (permalink / raw)
To: intel-gfx; +Cc: shobhit.kumar
DSP CLK_GATE registers are specific to BYT and CHT.
Avoid programming the same for BXT platform.
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_dsi.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 5fbcebe..59a52d1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -658,9 +658,11 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder)
intel_dsi_disable(encoder);
intel_dsi_clear_device_ready(encoder);
- val = I915_READ(DSPCLK_GATE_D);
- val &= ~DPOUNIT_CLOCK_GATE_DISABLE;
- I915_WRITE(DSPCLK_GATE_D, val);
+ if (!IS_BROXTON(dev)) {
+ val = I915_READ(DSPCLK_GATE_D);
+ val &= ~DPOUNIT_CLOCK_GATE_DISABLE;
+ I915_WRITE(DSPCLK_GATE_D, val);
+ }
drm_panel_unprepare(intel_dsi->panel);
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes
2015-05-22 16:06 ` [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes Uma Shankar
@ 2015-05-25 10:03 ` Jani Nikula
2015-05-25 16:57 ` Jani Nikula
0 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 10:03 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Sunil Kamath <sunil.kamath@intel.com>
>
> Latest VBT mentions which set of registers will be used for BLC,
> as controller number field. Making use of this field in BXT
> BLC implementation. Also, the registers are used in case control
> pin indicates display DDI. Adding a check for this.
> According to Bspec, BLC_PWM_*_2 uses the display utility pin for output.
> To use backlight 2, enable the utility pin with mode = PWM
> v2: Jani's review comments
> addressed
> - Add a prefix _ to BXT BLC registers definitions.
> - Add "bxt only" comment for u8 controller
> - Remove control_pin check for DDI controller
> - Check for valid controller values
> - Set pipe bits in UTIL_PIN_CTL
> - Enable/Disable UTIL_PIN_CTL in enable/disable_backlight()
> - If BLC 2 is used, read active_low_pwm from UTIL_PIN polarity
> Satheesh's review comment addressed
> - If UTIL PIN is already enabled, BIOS would have programmed it. No
> need to disable and enable again.
> v3: Jani's review comments
> - add UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK
> - Disable UTIL_PIN if controller 1 is used
> - Mask out UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK before enabling
> UTIL_PIN
> - check valid controller value in intel_bios.c
> - add backlight.util_pin_active_low
> - disable util pin before enabling
> v4: Change for BXT-PO branch:
> Stubbed unwanted definition which was existing before
> because of DC6 patch.
> UTIL_PIN_MODE_PWM (0x1b << 24)
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 28 +++++++---
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> drivers/gpu/drm/i915/intel_panel.c | 100 ++++++++++++++++++++++++++++++------
> 3 files changed, 106 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f96f049..5e3958f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3509,17 +3509,29 @@ enum skl_disp_power_wells {
> #define UTIL_PIN_CTL 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)
> +
> /* BXT backlight register definition. */
> -#define BXT_BLC_PWM_CTL1 0xC8250
> +#define _BXT_BLC_PWM_CTL1 0xC8250
> #define BXT_BLC_PWM_ENABLE (1 << 31)
> #define BXT_BLC_PWM_POLARITY (1 << 29)
> -#define BXT_BLC_PWM_FREQ1 0xC8254
> -#define BXT_BLC_PWM_DUTY1 0xC8258
> -
> -#define BXT_BLC_PWM_CTL2 0xC8350
> -#define BXT_BLC_PWM_FREQ2 0xC8354
> -#define BXT_BLC_PWM_DUTY2 0xC8358
> -
> +#define _BXT_BLC_PWM_FREQ1 0xC8254
> +#define _BXT_BLC_PWM_DUTY1 0xC8258
> +
> +#define _BXT_BLC_PWM_CTL2 0xC8350
> +#define _BXT_BLC_PWM_FREQ2 0xC8354
> +#define _BXT_BLC_PWM_DUTY2 0xC8358
> +
> +#define BXT_BLC_PWM_CTL(controller) _PIPE(controller, \
> + _BXT_BLC_PWM_CTL1, _BXT_BLC_PWM_CTL2)
> +#define BXT_BLC_PWM_FREQ(controller) _PIPE(controller, \
> + _BXT_BLC_PWM_FREQ1, _BXT_BLC_PWM_FREQ2)
> +#define BXT_BLC_PWM_DUTY(controller) _PIPE(controller, \
> + _BXT_BLC_PWM_DUTY1, _BXT_BLC_PWM_DUTY2)
>
> #define PCH_GTC_CTL 0xe7000
> #define PCH_GTC_ENABLE (1 << 31)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 47bc729..ee9eb2b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -182,7 +182,9 @@ struct intel_panel {
> bool enabled;
> bool combination_mode; /* gen 2/4 only */
> bool active_low_pwm;
> + bool util_pin_active_low; /* bxt+ */
> struct backlight_device *device;
> + u8 controller; /* bxt+ only */
> } backlight;
>
> void (*backlight_power)(struct intel_connector *, bool enable);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 7d83527..23847f2 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -32,7 +32,10 @@
>
> #include <linux/kernel.h>
> #include <linux/moduleparam.h>
> +#include <drm/drm_panel.h>
> #include "intel_drv.h"
> +#include "intel_dsi.h"
> +#include "i915_drv.h"
>
> void
> intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> @@ -539,9 +542,10 @@ static u32 vlv_get_backlight(struct intel_connector *connector)
> static u32 bxt_get_backlight(struct intel_connector *connector)
> {
> struct drm_device *dev = connector->base.dev;
> + struct intel_panel *panel = &connector->panel;
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - return I915_READ(BXT_BLC_PWM_DUTY1);
> + return I915_READ(BXT_BLC_PWM_DUTY(panel->backlight.controller));
> }
>
> static u32 intel_panel_get_backlight(struct intel_connector *connector)
> @@ -628,8 +632,9 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_panel *panel = &connector->panel;
>
> - I915_WRITE(BXT_BLC_PWM_DUTY1, level);
> + I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller), level);
> }
>
> static void
> @@ -761,12 +766,20 @@ static void bxt_disable_backlight(struct intel_connector *connector)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 tmp;
> + struct intel_panel *panel = &connector->panel;
> + u32 tmp, val;
>
> intel_panel_actually_set_backlight(connector, 0);
>
> - tmp = I915_READ(BXT_BLC_PWM_CTL1);
> - I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE);
> + tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> + tmp & ~BXT_BLC_PWM_ENABLE);
> +
> + if (panel->backlight.controller == 1) {
> + val = I915_READ(UTIL_PIN_CTL);
> + val &= ~UTIL_PIN_ENABLE;
> + I915_WRITE(UTIL_PIN_CTL, val);
> + }
> }
>
> void intel_panel_disable_backlight(struct intel_connector *connector)
> @@ -980,16 +993,39 @@ static void bxt_enable_backlight(struct intel_connector *connector)
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_panel *panel = &connector->panel;
> - u32 pwm_ctl;
> + enum pipe pipe = intel_get_pipe_from_connector(connector);
> + u32 pwm_ctl, val;
>
> - pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
> + /* To use 2nd set of backlight registers, utility pin has to be
> + * enabled with PWM mode.
> + * The field should only be changed when the utility pin is disabled
> + */
> + if (panel->backlight.controller == 1) {
> + val = I915_READ(UTIL_PIN_CTL);
> + if (val & UTIL_PIN_ENABLE) {
> + DRM_DEBUG_KMS("util pin already enabled\n");
> + val &= ~UTIL_PIN_ENABLE;
> + I915_WRITE(UTIL_PIN_CTL, val);
> + }
> + /* mask out UTIL_PIN_PIPE and UTIL_PIN_MODE */
> + val &= ~(UTIL_PIN_PIPE_MASK | UTIL_PIN_MODE_MASK);
> + I915_WRITE(UTIL_PIN_CTL, val);
> + if (panel->backlight.util_pin_active_low)
> + val |= UTIL_PIN_POLARITY;
> + I915_WRITE(UTIL_PIN_CTL, val | UTIL_PIN_PIPE(pipe) |
> + UTIL_PIN_MODE_PWM | UTIL_PIN_ENABLE);
> + }
> +
> + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> if (pwm_ctl & BXT_BLC_PWM_ENABLE) {
> DRM_DEBUG_KMS("backlight already enabled\n");
> pwm_ctl &= ~BXT_BLC_PWM_ENABLE;
> - I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> + pwm_ctl);
> }
>
> - I915_WRITE(BXT_BLC_PWM_FREQ1, panel->backlight.max);
> + I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
> + panel->backlight.max);
>
> intel_panel_actually_set_backlight(connector, panel->backlight.level);
>
> @@ -997,9 +1033,10 @@ static void bxt_enable_backlight(struct intel_connector *connector)
> if (panel->backlight.active_low_pwm)
> pwm_ctl |= BXT_BLC_PWM_POLARITY;
>
> - I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
> - POSTING_READ(BXT_BLC_PWM_CTL1);
> - I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE);
> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), pwm_ctl);
> + POSTING_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> + pwm_ctl | BXT_BLC_PWM_ENABLE);
> }
>
> void intel_panel_enable_backlight(struct intel_connector *connector)
> @@ -1008,6 +1045,9 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_panel *panel = &connector->panel;
> enum pipe pipe = intel_get_pipe_from_connector(connector);
> + struct intel_dsi *intel_dsi = NULL;
> + struct drm_crtc *crtc = NULL;
> + struct intel_encoder *encoder = NULL;
>
> if (!panel->backlight.present)
> return;
> @@ -1027,7 +1067,18 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
> panel->backlight.device->props.max_brightness);
> }
>
> - dev_priv->display.enable_backlight(connector);
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->type == INTEL_OUTPUT_DSI)
> + intel_dsi = enc_to_intel_dsi(&encoder->base);
> + }
> + }
> +
> + if (IS_BROXTON(dev) && intel_dsi->panel->funcs->enable)
> + intel_dsi->panel->funcs->enable(intel_dsi->panel);
> + else
> + dev_priv->display.enable_backlight(connector);
> +
This is backwards. The DSI code should call intel_panel_enable_backlight
and intel_panel_disable_backlight, not the other way round.
BR,
Jani.
> panel->backlight.enabled = true;
> if (panel->backlight.device)
> panel->backlight.device->props.power = FB_BLANK_UNBLANK;
> @@ -1362,10 +1413,27 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
> struct intel_panel *panel = &connector->panel;
> u32 pwm_ctl, val;
>
> - pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
> - panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
> + /* For BXT hard coding the Backlight controller to 0.
> + * ToDo : Read the controller value from VBT and generalize
> + */
> + panel->backlight.controller = 0;
> +
> + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> +
> + /* Keeping the check if controller 1 is to be programmed.
> + * This will come into affect once the VBT parsing
> + * is fixed for controller selection, and controller 1 is used
> + * for a prticular display configuration.
> + */
> + if (panel->backlight.controller == 1) {
> + val = I915_READ(UTIL_PIN_CTL);
> + panel->backlight.util_pin_active_low =
> + val & UTIL_PIN_POLARITY;
> + }
>
> - panel->backlight.max = I915_READ(BXT_BLC_PWM_FREQ1);
> + panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
> + panel->backlight.max = I915_READ(
> + BXT_BLC_PWM_FREQ(panel->backlight.controller));
> if (!panel->backlight.max)
> return -ENODEV;
>
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset
2015-05-22 16:05 ` [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset Uma Shankar
@ 2015-05-25 10:13 ` Jani Nikula
2015-05-25 11:24 ` Jani Nikula
2015-05-25 10:25 ` Jani Nikula
1 sibling, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 10:13 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> SKL and BXT qualifies the HAS_DDI() check, and hence haswell modeset
> functions are re-used for modeset sequence. But DDI interface doesn't
> include support for DSI.
> This patch adds:
> 1. cases for DSI encoder, in those modeset functions and allows a CRTC modeset
> 2. Adds call to pre_pll enabled from CRTC modeset function. Nothing needs to be
> done as such in CRTC for DSI encoder, as PLL, clock and and transcoder programming
> will be taken care in encoder's pre_enable and pre_pll_enable function.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/intel_display.c | 6 +++-
> drivers/gpu/drm/i915/intel_opregion.c | 1 +
> 4 files changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 840f08f..6874121 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2452,6 +2452,7 @@ struct drm_i915_cmd_table {
> INTEL_INFO(dev)->gen >= 9)
>
> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> +#define has_encoder_ddi(type) ((type) == (INTEL_OUTPUT_DSI) ? 0 : 1)
> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
> IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d602db2..2aef8b5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -395,6 +395,12 @@ void intel_prepare_ddi(struct drm_device *dev)
> if (visited[port])
> continue;
>
> + if (intel_dig_port->base.type ==
> + INTEL_OUTPUT_DSI) {
> + visited[intel_dig_port->port] = true;
> + continue;
> + }
> +
ddi_get_encoder_port does not support DSI, and DSI does not have
intel_digital_port.
Jani.
> supports_hdmi = intel_dig_port &&
> intel_dig_port_supports_hdmi(intel_dig_port);
>
> @@ -1568,10 +1574,21 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum pipe pipe = intel_crtc->pipe;
> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
> uint32_t temp;
>
> + /*
> + * Fixme: BXT has DDI, so tries to follow a DDI modeset function,
It's FIXME, TODO, or XXX, upper case. Grep must find these.
> + * but DDI interface doesn't support DSI yet, so don't do anything
> + * for DSI encoders
> + */
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
HAS_DDI() is always true
> + DRM_DEBUG_DRIVER("Not setting transcoder for DSI\n");
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> /* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
> temp = TRANS_DDI_FUNC_ENABLE;
> temp |= TRANS_DDI_SELECT_PORT(port);
> @@ -1779,11 +1796,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> {
> struct drm_crtc *crtc = &intel_crtc->base;
> - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> + int type = intel_encoder->type;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type)))
> + return;
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> if (cpu_transcoder != TRANSCODER_EDP)
> I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> TRANS_CLK_SEL_PORT(port));
> @@ -1866,10 +1889,16 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
> int hdmi_level;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
> + DRM_ERROR("DDI func getting called for MIPI?\n");
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> if (type == INTEL_OUTPUT_EDP) {
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> intel_edp_panel_on(intel_dp);
> @@ -1942,11 +1971,17 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> struct drm_encoder *encoder = &intel_encoder->base;
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
> uint32_t val;
> bool wait = false;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
> + DRM_ERROR("DDI func got called for DSI?\n");
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> val = I915_READ(DDI_BUF_CTL(port));
> if (val & DDI_BUF_CTL_ENABLE) {
> val &= ~DDI_BUF_CTL_ENABLE;
> @@ -1983,9 +2018,15 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
> + DRM_ERROR("DDI func getting called for DSI?\n");
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> if (type == INTEL_OUTPUT_HDMI) {
> struct intel_digital_port *intel_dig_port =
> enc_to_dig_port(encoder);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c30bfd4..c1fac21 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4987,6 +4987,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>
> WARN_ON(!crtc->state->enable);
>
> @@ -5052,13 +5053,16 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> if (intel_crtc->config->has_pch_encoder)
> lpt_pch_enable(crtc);
>
> - if (intel_crtc->config->dp_encoder_is_mst)
> + if (intel_crtc->config->dp_encoder_is_mst && !is_dsi)
> intel_ddi_set_vc_payload_alloc(crtc, true);
>
> assert_vblank_disabled(crtc);
> drm_crtc_vblank_on(crtc);
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->pre_pll_enable)
> + encoder->pre_pll_enable(encoder);
> +
> encoder->enable(encoder);
> intel_opregion_notify_encoder(encoder, true);
> }
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 71e87ab..4b025ee 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -356,6 +356,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
> break;
> case INTEL_OUTPUT_EDP:
> + case INTEL_OUTPUT_DSI:
> type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
> break;
> default:
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset
2015-05-22 16:05 ` [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset Uma Shankar
2015-05-25 10:13 ` Jani Nikula
@ 2015-05-25 10:25 ` Jani Nikula
2015-05-26 7:11 ` Daniel Vetter
1 sibling, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 10:25 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> SKL and BXT qualifies the HAS_DDI() check, and hence haswell modeset
> functions are re-used for modeset sequence. But DDI interface doesn't
> include support for DSI.
> This patch adds:
> 1. cases for DSI encoder, in those modeset functions and allows a CRTC modeset
> 2. Adds call to pre_pll enabled from CRTC modeset function. Nothing needs to be
> done as such in CRTC for DSI encoder, as PLL, clock and and transcoder programming
> will be taken care in encoder's pre_enable and pre_pll_enable function.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/intel_display.c | 6 +++-
> drivers/gpu/drm/i915/intel_opregion.c | 1 +
> 4 files changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 840f08f..6874121 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2452,6 +2452,7 @@ struct drm_i915_cmd_table {
> INTEL_INFO(dev)->gen >= 9)
>
> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> +#define has_encoder_ddi(type) ((type) == (INTEL_OUTPUT_DSI) ? 0 : 1)
> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
> IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d602db2..2aef8b5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -395,6 +395,12 @@ void intel_prepare_ddi(struct drm_device *dev)
> if (visited[port])
> continue;
>
> + if (intel_dig_port->base.type ==
> + INTEL_OUTPUT_DSI) {
> + visited[intel_dig_port->port] = true;
> + continue;
> + }
> +
ddi_get_encoder_port does not support DSI, and DSI does not have
intel_digital_port.
> supports_hdmi = intel_dig_port &&
> intel_dig_port_supports_hdmi(intel_dig_port);
>
> @@ -1568,10 +1574,21 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum pipe pipe = intel_crtc->pipe;
> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
> uint32_t temp;
>
> + /*
> + * Fixme: BXT has DDI, so tries to follow a DDI modeset function,
It's FIXME, TODO, or XXX, upper case. Grep must find these.
> + * but DDI interface doesn't support DSI yet, so don't do anything
> + * for DSI encoders
> + */
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
HAS_DDI() is always true here.
Hmm. Perhaps it would be nicer if we added INVALID_PORT = -1 to enum
port, and had intel_ddi_get_encoder_port() return that for DSI. Then we
could leave most of the functions the same, with just
if (port == INVALID_PORT)
return;
at the beginning.
Daniel, opinions?
> + DRM_DEBUG_DRIVER("Not setting transcoder for DSI\n");
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> /* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
> temp = TRANS_DDI_FUNC_ENABLE;
> temp |= TRANS_DDI_SELECT_PORT(port);
> @@ -1779,11 +1796,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> {
> struct drm_crtc *crtc = &intel_crtc->base;
> - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> + int type = intel_encoder->type;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type)))
> + return;
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> if (cpu_transcoder != TRANSCODER_EDP)
> I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> TRANS_CLK_SEL_PORT(port));
> @@ -1866,10 +1889,16 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
> int hdmi_level;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
> + DRM_ERROR("DDI func getting called for MIPI?\n");
I don't think it's worth having these checks for pure DDI encoder
callback funcs. If we started on this path, our codebase would be filled
with this. Only do the DSI check on functions called from outside of
intel_ddi.c.
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> if (type == INTEL_OUTPUT_EDP) {
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> intel_edp_panel_on(intel_dp);
> @@ -1942,11 +1971,17 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> struct drm_encoder *encoder = &intel_encoder->base;
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
> uint32_t val;
> bool wait = false;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
> + DRM_ERROR("DDI func got called for DSI?\n");
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> val = I915_READ(DDI_BUF_CTL(port));
> if (val & DDI_BUF_CTL_ENABLE) {
> val &= ~DDI_BUF_CTL_ENABLE;
> @@ -1983,9 +2018,15 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
> + enum port port;
> int type = intel_encoder->type;
>
> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
> + DRM_ERROR("DDI func getting called for DSI?\n");
> + return;
> + }
> +
> + port = intel_ddi_get_encoder_port(intel_encoder);
> if (type == INTEL_OUTPUT_HDMI) {
> struct intel_digital_port *intel_dig_port =
> enc_to_dig_port(encoder);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c30bfd4..c1fac21 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4987,6 +4987,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>
> WARN_ON(!crtc->state->enable);
>
> @@ -5052,13 +5053,16 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> if (intel_crtc->config->has_pch_encoder)
> lpt_pch_enable(crtc);
>
> - if (intel_crtc->config->dp_encoder_is_mst)
> + if (intel_crtc->config->dp_encoder_is_mst && !is_dsi)
> intel_ddi_set_vc_payload_alloc(crtc, true);
>
> assert_vblank_disabled(crtc);
> drm_crtc_vblank_on(crtc);
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->pre_pll_enable)
> + encoder->pre_pll_enable(encoder);
> +
> encoder->enable(encoder);
> intel_opregion_notify_encoder(encoder, true);
> }
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 71e87ab..4b025ee 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -356,6 +356,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
> break;
> case INTEL_OUTPUT_EDP:
> + case INTEL_OUTPUT_DSI:
You would have oopsed in the intel_ddi_get_encoder_port already before
reaching here...
> type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
> break;
> default:
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset
2015-05-25 10:13 ` Jani Nikula
@ 2015-05-25 11:24 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 11:24 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
Please disregard this one, sent prematurely. Sorry.
On Mon, 25 May 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
>> From: Shashank Sharma <shashank.sharma@intel.com>
>>
>> SKL and BXT qualifies the HAS_DDI() check, and hence haswell modeset
>> functions are re-used for modeset sequence. But DDI interface doesn't
>> include support for DSI.
>> This patch adds:
>> 1. cases for DSI encoder, in those modeset functions and allows a CRTC modeset
>> 2. Adds call to pre_pll enabled from CRTC modeset function. Nothing needs to be
>> done as such in CRTC for DSI encoder, as PLL, clock and and transcoder programming
>> will be taken care in encoder's pre_enable and pre_pll_enable function.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++++++++++++----
>> drivers/gpu/drm/i915/intel_display.c | 6 +++-
>> drivers/gpu/drm/i915/intel_opregion.c | 1 +
>> 4 files changed, 54 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 840f08f..6874121 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2452,6 +2452,7 @@ struct drm_i915_cmd_table {
>> INTEL_INFO(dev)->gen >= 9)
>>
>> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
>> +#define has_encoder_ddi(type) ((type) == (INTEL_OUTPUT_DSI) ? 0 : 1)
>> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
>> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
>> IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index d602db2..2aef8b5 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -395,6 +395,12 @@ void intel_prepare_ddi(struct drm_device *dev)
>> if (visited[port])
>> continue;
>>
>> + if (intel_dig_port->base.type ==
>> + INTEL_OUTPUT_DSI) {
>> + visited[intel_dig_port->port] = true;
>> + continue;
>> + }
>> +
>
> ddi_get_encoder_port does not support DSI, and DSI does not have
> intel_digital_port.
>
> Jani.
>
>> supports_hdmi = intel_dig_port &&
>> intel_dig_port_supports_hdmi(intel_dig_port);
>>
>> @@ -1568,10 +1574,21 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> enum pipe pipe = intel_crtc->pipe;
>> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
>> + enum port port;
>> int type = intel_encoder->type;
>> uint32_t temp;
>>
>> + /*
>> + * Fixme: BXT has DDI, so tries to follow a DDI modeset function,
>
> It's FIXME, TODO, or XXX, upper case. Grep must find these.
>
>> + * but DDI interface doesn't support DSI yet, so don't do anything
>> + * for DSI encoders
>> + */
>> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
>
> HAS_DDI() is always true
>
>
>> + DRM_DEBUG_DRIVER("Not setting transcoder for DSI\n");
>> + return;
>> + }
>> +
>> + port = intel_ddi_get_encoder_port(intel_encoder);
>> /* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
>> temp = TRANS_DDI_FUNC_ENABLE;
>> temp |= TRANS_DDI_SELECT_PORT(port);
>> @@ -1779,11 +1796,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>> void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
>> {
>> struct drm_crtc *crtc = &intel_crtc->base;
>> - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>> + struct drm_device *dev = crtc->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
>> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
>> + enum port port;
>> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>> + int type = intel_encoder->type;
>>
>> + if (!(HAS_DDI(dev) && has_encoder_ddi(type)))
>> + return;
>> +
>> + port = intel_ddi_get_encoder_port(intel_encoder);
>> if (cpu_transcoder != TRANSCODER_EDP)
>> I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
>> TRANS_CLK_SEL_PORT(port));
>> @@ -1866,10 +1889,16 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>> struct drm_device *dev = encoder->dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
>> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
>> + enum port port;
>> int type = intel_encoder->type;
>> int hdmi_level;
>>
>> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
>> + DRM_ERROR("DDI func getting called for MIPI?\n");
>> + return;
>> + }
>> +
>> + port = intel_ddi_get_encoder_port(intel_encoder);
>> if (type == INTEL_OUTPUT_EDP) {
>> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>> intel_edp_panel_on(intel_dp);
>> @@ -1942,11 +1971,17 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>> struct drm_encoder *encoder = &intel_encoder->base;
>> struct drm_device *dev = encoder->dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
>> + enum port port;
>> int type = intel_encoder->type;
>> uint32_t val;
>> bool wait = false;
>>
>> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
>> + DRM_ERROR("DDI func got called for DSI?\n");
>> + return;
>> + }
>> +
>> + port = intel_ddi_get_encoder_port(intel_encoder);
>> val = I915_READ(DDI_BUF_CTL(port));
>> if (val & DDI_BUF_CTL_ENABLE) {
>> val &= ~DDI_BUF_CTL_ENABLE;
>> @@ -1983,9 +2018,15 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> struct drm_device *dev = encoder->dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> - enum port port = intel_ddi_get_encoder_port(intel_encoder);
>> + enum port port;
>> int type = intel_encoder->type;
>>
>> + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
>> + DRM_ERROR("DDI func getting called for DSI?\n");
>> + return;
>> + }
>> +
>> + port = intel_ddi_get_encoder_port(intel_encoder);
>> if (type == INTEL_OUTPUT_HDMI) {
>> struct intel_digital_port *intel_dig_port =
>> enc_to_dig_port(encoder);
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index c30bfd4..c1fac21 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4987,6 +4987,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> struct intel_encoder *encoder;
>> int pipe = intel_crtc->pipe;
>> + bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
>>
>> WARN_ON(!crtc->state->enable);
>>
>> @@ -5052,13 +5053,16 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>> if (intel_crtc->config->has_pch_encoder)
>> lpt_pch_enable(crtc);
>>
>> - if (intel_crtc->config->dp_encoder_is_mst)
>> + if (intel_crtc->config->dp_encoder_is_mst && !is_dsi)
>> intel_ddi_set_vc_payload_alloc(crtc, true);
>>
>> assert_vblank_disabled(crtc);
>> drm_crtc_vblank_on(crtc);
>>
>> for_each_encoder_on_crtc(dev, crtc, encoder) {
>> + if (encoder->pre_pll_enable)
>> + encoder->pre_pll_enable(encoder);
>> +
>> encoder->enable(encoder);
>> intel_opregion_notify_encoder(encoder, true);
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>> index 71e87ab..4b025ee 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -356,6 +356,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>> type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
>> break;
>> case INTEL_OUTPUT_EDP:
>> + case INTEL_OUTPUT_DSI:
>> type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
>> break;
>> default:
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/12] drm/i915/bxt: Enable BXT DSI PLL
2015-05-22 16:05 ` [PATCH 02/12] drm/i915/bxt: Enable BXT DSI PLL Uma Shankar
@ 2015-05-25 16:10 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:10 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> This patch adds new functions for BXT clock and PLL programming.
> They are:
> 1. configure_dsi_pll for BXT.
> This function does the basic math and generates the divider ratio
> based on requested pixclock, and program clock registers.
> 2. enable_dsi_pll function.
> This function programs the calculated clock values on the PLL.
> 3. intel_enable_dsi_pll
> Wrapper function to use same code for multiple platforms. It checks the
> platform and calls appropriate core pll enable function.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 35 ++++++++++++++
> drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> drivers/gpu/drm/i915/intel_dsi.h | 2 +-
> drivers/gpu/drm/i915/intel_dsi_pll.c | 85 +++++++++++++++++++++++++++++++++-
> 4 files changed, 121 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 46ef269..b63bdce 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7352,6 +7352,41 @@ enum skl_disp_power_wells {
>
> /* MIPI DSI registers */
>
> +/* BXT DSI PLL Control */
The macro name says this already, no need for the comment.
> +#define BXT_DSI_PLL_CTL 0x161000
> +#define BXT_DSI_PLL_MASK_PVD_RATIO (3 << 16)
The convention is to have _SHIFT and _MASK at the end of the name.
> +#define BXT_DSI_PLL_PVD_RATIO_1 (1<<16)
Spaces around <<. Please also add macro for _SHIFT.
> +#define BXT_DSIC_16X_BY2 (1 << 10)
> +#define BXT_DSIC_16X_BY3 (1 << 11)
> +#define BXT_DSIC_16X_BY4 (3 << 10)
The convention is to shift the same amount for each alternative in a bit
field. E.g. (1 << 10), (2 << 10), (3 << 10).
> +#define BXT_DSIA_16X_BY2 (1 << 8)
> +#define BXT_DSIA_16X_BY3 (1 << 9)
> +#define BXT_DSIA_16X_BY4 (3 << 8)
Same.
> +#define BXT_DSI_MASK_FREQ_SEL (0xF << 8)
_MASK at the end of the name, and the convention is to define _SHIFT
first, _MASK next, and the possible alternatives after that, i.e.
#define FOO_SHIFT 16
#define FOO_MASK (3 << 16)
#define FOO_A (1 << 16)
> +#define BXT_DSI_PLL_RATIO_MAX 0x7D
> +#define BXT_DSI_PLL_RATIO_MIN 0x22
> +#define BXT_DSI_MASK_PLL_RATIO 0x7F
Same as above. The mask is 8 bits, i.e. 0xff.
The convention is to have bit names shifted by two spaces. For example,
#define REGISTER_NAME 0x50000
#define SOME_BIT (1 << 5)
And please no empty lines between bit names, group them together.
> +#define BXT_REF_CLOCK_KHZ 19500
> +
> +#define BXT_DSI_PLL_ENABLE 0x46080
> +#define BXT_DSI_PLL_DO_ENABLE (1 << 31)
> +#define BXT_DSI_PLL_LOCKED (1 << 30)
> +
> +/* BXT power well control2, for driver */
The macro name says this already, no need for the comment.
> +#define BXT_PWR_WELL_CTL2 0x45404
> +#define BXT_PWR_WELL_2_ENABLE (1 << 31)
> +#define BXT_PWR_WELL_2_ENABLED (1 << 30)
> +#define BXT_PWR_WELL_1_ENABLE (1 << 29)
> +#define BXT_PWR_WELL_1_ENABLED (1 << 28)
> +#define BXT_PWR_WELL_ENABLED (BXT_PWR_WELL_1_ENABLED | \
> + BXT_PWR_WELL_2_ENABLED)
> +#define GEN9_FUSE_STATUS 0x42000
> +#define GEN9_FUSE_PG2_ENABLED (1 << 25)
> +#define GEN9_FUSE_PG1_ENABLED (1 << 26)
> +#define GEN9_FUSE_PG0_ENABLED (1 << 27)
> +
BXT_PWR_WELL_CTL2 and GEN9_FUSE_STATUS are not used in this series at
all, please remove them from this patch (and perhaps send them in a
separate patch).
> #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */
>
> #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 2419929e..1927546 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -903,8 +903,8 @@ static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
> DRM_DEBUG_KMS("\n");
>
> intel_dsi_prepare(encoder);
> + intel_enable_dsi_pll(encoder);
>
> - vlv_enable_dsi_pll(encoder);
> }
>
> static enum drm_connector_status
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 2784ac4..20cfcf07 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -121,7 +121,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
> return container_of(encoder, struct intel_dsi, base.base);
> }
>
> -extern void vlv_enable_dsi_pll(struct intel_encoder *encoder);
> +extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
> extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
> extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 9688d99..a58f0a1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -237,7 +237,7 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
> vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, dsi_mnp.dsi_pll_ctrl);
> }
>
> -void vlv_enable_dsi_pll(struct intel_encoder *encoder)
> +static void vlv_enable_dsi_pll(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> u32 tmp;
> @@ -368,3 +368,86 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
>
> return pclk;
> }
> +
> +static bool bxt_configure_dsi_pll(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + u8 dsi_ratio;
> + u32 dsi_clk;
> + u32 val = 0;
Unnecessary initialization.
> +
> + dsi_clk = dsi_clk_from_pclk(intel_dsi->pclk, intel_dsi->pixel_format,
> + intel_dsi->lane_count);
> +
> + /*
> + * From clock diagram, to get PLL ratio divider, divide double of DSI
> + * link rate (i.e., 2*8x=16x frequency value) by ref clock. Make sure to
> + * round 'up' the result
> + */
> + dsi_ratio = DIV_ROUND_UP(dsi_clk * 2, BXT_REF_CLOCK_KHZ);
> + if (dsi_ratio < BXT_DSI_PLL_RATIO_MIN ||
> + dsi_ratio > BXT_DSI_PLL_RATIO_MAX) {
> + DRM_ERROR("Cant get a suitable ratio from DSI PLL ratios\n");
> + return false;
> + }
> +
> + /*
> + * Program DSI ratio and Select MIPIC and MIPIA PLL output as 8x
> + * Spec says both have to be programmed, even if one is not getting
> + * used. Configure MIPI_CLOCK_CTL dividers in modeset
> + */
> + val = I915_READ(BXT_DSI_PLL_CTL);
> + val &= ~BXT_DSI_PLL_MASK_PVD_RATIO;
> + val &= ~BXT_DSI_MASK_FREQ_SEL;
> + val &= ~BXT_DSI_MASK_PLL_RATIO;
> + val |= (dsi_ratio | BXT_DSIA_16X_BY2 | BXT_DSIC_16X_BY2);
> +
> + /* Prog PVD ratio =1 if ratio <= 50 */
I can read that from the code. The comments are for telling the reader
*why* this is being done. What's the magic 50?
> + if (dsi_ratio <= 50) {
> + val &= ~BXT_DSI_PLL_MASK_PVD_RATIO;
> + val |= BXT_DSI_PLL_PVD_RATIO_1;
> + }
> +
> + I915_WRITE(BXT_DSI_PLL_CTL, val);
Hmm, POSTING_READ?
> + return true;
> +}
> +
> +static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 val = 0;
Unnecessary initialization, the first thing you do with val is assign to
it.
> +
> + DRM_DEBUG_KMS("\n");
> +
Perhaps you should check if DSI PLL is enabled here, and disable with a
warning if it is?
> + /* Configure PLL vales */
> + if (!bxt_configure_dsi_pll(encoder)) {
> + DRM_ERROR("Configure DSI PLL failed, abort PLL enable\n");
> + return;
> + }
> +
> + /* Enable DSI PLL */
> + val = I915_READ(BXT_DSI_PLL_ENABLE);
> + val |= BXT_DSI_PLL_DO_ENABLE;
> + I915_WRITE(BXT_DSI_PLL_ENABLE, val);
> +
> + /* Timeout and fail if PLL not locked */
> + if (wait_for(I915_READ(BXT_DSI_PLL_ENABLE) & BXT_DSI_PLL_LOCKED, 1)) {
> + DRM_ERROR("DSI PLL not locked\n");
> + return;
> + }
> +
> + DRM_DEBUG_KMS("DSI PLL locked\n");
> +}
> +
> +void intel_enable_dsi_pll(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> +
> + if (IS_VALLEYVIEW(dev))
> + vlv_enable_dsi_pll(encoder);
> + else if (IS_BROXTON(dev))
> + bxt_enable_dsi_pll(encoder);
> + else
> + DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
Please drop all such else branches. It's already evident that having a
few of them around leads to more and more added, and there is no end to
it.
> +}
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/12] drm/i915/bxt: Disable DSI PLL for BXT
2015-05-22 16:05 ` [PATCH 03/12] drm/i915/bxt: Disable DSI PLL for BXT Uma Shankar
@ 2015-05-25 16:12 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:12 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> This patch adds two new functions:
> - disable_dsi_pll.
> BXT DSI disable sequence and registers are
> different from previous platforms.
> - intel_disable_dsi_pll
> wrapper function to re-use the same code for
> multiple platforms. It checks platform type and
> calls appropriate core pll disable function.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> drivers/gpu/drm/i915/intel_dsi.h | 2 +-
> drivers/gpu/drm/i915/intel_dsi_pll.c | 31 ++++++++++++++++++++++++++++++-
> 3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 1927546..2c0ea6f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -553,7 +553,7 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
> usleep_range(2000, 2500);
> }
>
> - vlv_disable_dsi_pll(encoder);
> + intel_disable_dsi_pll(encoder);
> }
>
> static void intel_dsi_post_disable(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 20cfcf07..759983e 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -122,7 +122,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
> }
>
> extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
> -extern void vlv_disable_dsi_pll(struct intel_encoder *encoder);
> +extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
> extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
>
> struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index a58f0a1..0cbcf32 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -267,7 +267,7 @@ static void vlv_enable_dsi_pll(struct intel_encoder *encoder)
> DRM_DEBUG_KMS("DSI PLL locked\n");
> }
>
> -void vlv_disable_dsi_pll(struct intel_encoder *encoder)
> +static void vlv_disable_dsi_pll(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> u32 tmp;
> @@ -440,6 +440,23 @@ static void bxt_enable_dsi_pll(struct intel_encoder *encoder)
> DRM_DEBUG_KMS("DSI PLL locked\n");
> }
>
> +static void bxt_disable_dsi_pll(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* Disable DSI PLL */
That is obvious from the code.
> + I915_WRITE(BXT_DSI_PLL_ENABLE, 0);
> +
> + /* Should timeout and fail if not locked after 200 us */
> + if (wait_for((I915_READ(BXT_DSI_PLL_ENABLE)
> + & BXT_DSI_PLL_LOCKED) == 0, 1)) {
> + DRM_ERROR("Disable: DSI PLL not locked\n");
not unlocked?
> + return;
Unnecessary return statement.
> + }
> +}
> +
> void intel_enable_dsi_pll(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -451,3 +468,15 @@ void intel_enable_dsi_pll(struct intel_encoder *encoder)
> else
> DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
> }
> +
> +void intel_disable_dsi_pll(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> +
> + if (IS_VALLEYVIEW(dev))
> + vlv_disable_dsi_pll(encoder);
> + else if (IS_BROXTON(dev))
> + bxt_disable_dsi_pll(encoder);
> + else
> + DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
Please drop the else branch.
BR,
Jani.
> +}
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 04/12] drm/i915/bxt: DSI prepare changes for BXT
2015-05-22 16:05 ` [PATCH 04/12] drm/i915/bxt: DSI prepare changes " Uma Shankar
@ 2015-05-25 16:25 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:25 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> This patch modifies dsi_prepare() function to support the same
> modeset prepare sequence for BXT also. Main changes are:
> 1. BXT port control register is different than VLV.
> 2. BXT modeset sequence needs vdisplay and hdisplay programmed
> for transcoder.
> 3. BXT can select PIPE for MIPI transcoders.
> 4. BXT needs to program register MIPI_INIT_COUNT for both the ports,
> even if only one is being used.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 22 +++++++++++
> drivers/gpu/drm/i915/intel_dsi.c | 79 +++++++++++++++++++++++++++++++++-----
> 2 files changed, 91 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b63bdce..57c3a48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7387,6 +7387,22 @@ enum skl_disp_power_wells {
> #define GEN9_FUSE_PG1_ENABLED (1 << 26)
> #define GEN9_FUSE_PG0_ENABLED (1 << 27)
>
> +/* BXT MIPI mode configure */
> +#define _BXT_MIPIA_TRANS_HACTIVE 0x6B0F8
> +#define _BXT_MIPIC_TRANS_HACTIVE 0x6B8F8
> +#define BXT_MIPI_TRANS_HACTIVE(tc) _TRANSCODER(tc, \
> + _BXT_MIPIA_TRANS_HACTIVE, _BXT_MIPIC_TRANS_HACTIVE)
> +
> +#define _BXT_MIPIA_TRANS_VACTIVE 0x6B0FC
> +#define _BXT_MIPIC_TRANS_VACTIVE 0x6B8FC
> +#define BXT_MIPI_TRANS_VACTIVE(tc) _TRANSCODER(tc, \
> + _BXT_MIPIA_TRANS_VACTIVE, _BXT_MIPIC_TRANS_VACTIVE)
> +
> +#define _BXT_MIPIA_TRANS_VTOTAL 0x6B100
> +#define _BXT_MIPIC_TRANS_VTOTAL 0x6B900
> +#define BXT_MIPI_TRANS_VTOTAL(tc) _TRANSCODER(tc, \
> + _BXT_MIPIA_TRANS_VTOTAL, _BXT_MIPIC_TRANS_VTOTAL)
Shouldn't these use _MIPI_PORT, not _TRANSCODER?
> +
> #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */
>
> #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
> @@ -7802,6 +7818,12 @@ enum skl_disp_power_wells {
> #define READ_REQUEST_PRIORITY_HIGH (3 << 3)
> #define RGB_FLIP_TO_BGR (1 << 2)
>
> +/* BXT has a pipe select field */
It's obvious from the defines. Please put the defines in decreasing bit
shift order, i.e. these will be the first ones.
> +#define BXT_PIPE_SELECT_A (0 << 7)
> +#define BXT_PIPE_SELECT_B (1 << 7)
> +#define BXT_PIPE_SELECT_C (2 << 7)
> +#define BXT_PIPE_SELECT_MASK (7 << 7)
Two spaces after "#define". _MASK goes first.
> +
> #define _MIPIA_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb108)
> #define _MIPIC_DATA_ADDRESS (dev_priv->mipi_mmio_base + 0xb908)
> #define MIPI_DATA_ADDRESS(port) _MIPI_PORT(port, _MIPIA_DATA_ADDRESS, \
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 2c0ea6f..892b518 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -726,6 +726,21 @@ static void set_dsi_timings(struct drm_encoder *encoder,
> hbp = txbyteclkhs(hbp, bpp, lane_count, intel_dsi->burst_mode_ratio);
>
> for_each_dsi_port(port, intel_dsi->ports) {
> + if (IS_BROXTON(dev)) {
> + /*
> + * Program hdisplay and vdisplay on MIPI transcoder.
> + * This is different from calculated hactive and
> + * vactive, as they are calculated per channel basis,
> + * whereas these values should be based on resolution.
> + */
> + I915_WRITE(BXT_MIPI_TRANS_HACTIVE(port),
> + mode->hdisplay);
> + I915_WRITE(BXT_MIPI_TRANS_VACTIVE(port),
> + mode->vdisplay);
> + I915_WRITE(BXT_MIPI_TRANS_VTOTAL(port),
> + mode->vtotal);
With the current definition of BXT_MIPI_TRANS_* these will fail for port
C.
> + }
> +
> I915_WRITE(MIPI_HACTIVE_AREA_COUNT(port), hactive);
> I915_WRITE(MIPI_HFP_COUNT(port), hfp);
>
> @@ -766,16 +781,38 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
> }
>
> for_each_dsi_port(port, intel_dsi->ports) {
> - /* escape clock divider, 20MHz, shared for A and C.
> - * device ready must be off when doing this! txclkesc? */
> - tmp = I915_READ(MIPI_CTRL(PORT_A));
> - tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
> - I915_WRITE(MIPI_CTRL(PORT_A), tmp | ESCAPE_CLOCK_DIVIDER_1);
> -
> - /* read request priority is per pipe */
> - tmp = I915_READ(MIPI_CTRL(port));
> - tmp &= ~READ_REQUEST_PRIORITY_MASK;
> - I915_WRITE(MIPI_CTRL(port), tmp | READ_REQUEST_PRIORITY_HIGH);
> + if (IS_VALLEYVIEW(dev)) {
> + /*
> + * escape clock divider, 20MHz, shared for A and C.
> + * device ready must be off when doing this! txclkesc?
> + */
> + tmp = I915_READ(MIPI_CTRL(0));
> + tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
> + I915_WRITE(MIPI_CTRL(0), tmp |
> + ESCAPE_CLOCK_DIVIDER_1);
> +
> + /* read request priority is per pipe */
> + tmp = I915_READ(MIPI_CTRL(port));
> + tmp &= ~READ_REQUEST_PRIORITY_MASK;
> + I915_WRITE(MIPI_CTRL(port), tmp |
> + READ_REQUEST_PRIORITY_HIGH);
> + } else if (IS_BROXTON(dev)) {
> + /*
> + * FIXME:
> + * BXT can connect any PIPE to any MIPI transcoder.
Do you mean, to any DSI port?
> + * Select the pipe based on the MIPI port read from
> + * VBT for now. Pick PIPE A for MIPI port A and C
> + * for port C.
> + */
> + tmp = I915_READ(MIPI_CTRL(port));
> + tmp &= ~BXT_PIPE_SELECT_MASK;
> + port ? (tmp |= BXT_PIPE_SELECT_C) :
> + (tmp |= BXT_PIPE_SELECT_A);
Please compare with PORT_* explicitly instead of port != 0.
> + I915_WRITE(MIPI_CTRL(port), tmp);
> + } else {
> + DRM_ERROR("Invalid DSI device to prepare seq\n");
> + return;
> + }
Please drop all such else branches.
>
> /* XXX: why here, why like this? handling in irq handler?! */
> I915_WRITE(MIPI_INTR_STAT(port), 0xffffffff);
> @@ -852,6 +889,28 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
> I915_WRITE(MIPI_INIT_COUNT(port),
> txclkesc(intel_dsi->escape_clk_div, 100));
>
> + if (IS_BROXTON(dev)) {
> +
> + /*
> + * XXX:
> + * PIPE2D says LOW_CONTENTION_RECOVERY_DISABLE is
> + * reqd for BXT, but bspec doesn't mention it.
> + * Lets follow PIPE2D for now.
> + */
> + val |= LOW_CONTENTION_RECOVERY_DISABLE;
> +
> + if (!intel_dsi->dual_link) {
> + /*
> + * BXT spec says write MIPI_INIT_COUNT for
> + * both the ports, even if only one is
> + * getting used. So write the other port
> + * if not in dual link mode.
> + */
> + I915_WRITE(MIPI_INIT_COUNT(port ==
> + PORT_A ? PORT_C : PORT_A),
> + intel_dsi->init_count);
Hmm, why do you write different things to the other port? See above what
gets written normally.
BR,
Jani.
> + }
> + }
>
> /* recovery disables */
> I915_WRITE(MIPI_EOT_DISABLE(port), tmp);
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 06/12] drm/i915/bxt: DSI enable for BXT
2015-05-22 16:05 ` [PATCH 06/12] drm/i915/bxt: DSI enable for BXT Uma Shankar
@ 2015-05-25 16:39 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:39 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> This patch contains following changes:
> 1. MIPI device ready changes to support dsi_pre_enable. Changes
> are specific to BXT device ready sequence. Added check for
> ULPS mode(No effects on VLV).
> 2. Changes in dsi_enable to pick BXT port control register.
> 3. Changes in dsi_pre_enable to restrict DPIO programming for VLV
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 ++
> drivers/gpu/drm/i915/intel_dsi.c | 185 ++++++++++++++++++++++++++++----------
> 2 files changed, 144 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 57c3a48..5eeb2b7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7408,6 +7408,13 @@ enum skl_disp_power_wells {
> #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190)
> #define _MIPIC_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700)
> #define MIPI_PORT_CTRL(port) _MIPI_PORT(port, _MIPIA_PORT_CTRL, _MIPIC_PORT_CTRL)
> +
> + /* BXT port control */
> +#define _BXT_MIPIA_PORT_CTRL 0x6B0C0
> +#define _BXT_MIPIC_PORT_CTRL 0x6B8C0
> +#define BXT_MIPI_PORT_CTRL(tc) _TRANSCODER(tc, _BXT_MIPIA_PORT_CTRL, \
> + _BXT_MIPIC_PORT_CTRL)
Shouldn't this use _MIPI_PORT, not _TRANSCODER?
> +
> #define DPI_ENABLE (1 << 31) /* A + C */
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
> #define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 892b518..3a1bb04 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -286,6 +286,101 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
> return true;
> }
>
> +static void bxt_dsi_device_ready(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + enum port port;
> + u32 val;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + /* Exit Low power state in 4 steps*/
> + for_each_dsi_port(port, intel_dsi->ports) {
> +
> + /* 1. Enable MIPI PHY transparent latch */
> + val = I915_READ(BXT_MIPI_PORT_CTRL(port));
> + I915_WRITE(BXT_MIPI_PORT_CTRL(port), val | LP_OUTPUT_HOLD);
> + usleep_range(2000, 2500);
> +
> + /* 2. Enter ULPS */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= (ULPS_STATE_ENTER | DEVICE_READY);
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> + usleep_range(2, 3);
> +
> + /* 3. Exit ULPS */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= (ULPS_STATE_EXIT | DEVICE_READY);
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> + usleep_range(1000, 1500);
> +
> + /* Clear ULPS and set device ready */
> + val = I915_READ(MIPI_DEVICE_READY(port));
> + val &= ~ULPS_STATE_MASK;
> + val |= DEVICE_READY;
> + I915_WRITE(MIPI_DEVICE_READY(port), val);
> + }
> +}
> +
> +static void vlv_dsi_device_ready(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + int pipe = intel_crtc->pipe;
enum pipe.
> + u32 val;
> + int count = 1;
> + u32 reg = 0;
> + bool afe_latchout = true;
> +
> + DRM_DEBUG_KMS("\n");
> +
> + pipe = intel_crtc->pipe;
> + reg = MIPI_PORT_CTRL(pipe);
Take care to not use pipe for port. Passing PIPE_B as port will *not*
use PORT_C.
> +
> + /* Ceck LP state */
> + val = I915_READ(reg);
> + afe_latchout = (val & AFE_LATCHOUT);
> +
> + /* Enter low power state, if not already in */
> + if (afe_latchout) {
> + /* Enter ULPS, wait for 2.5 ms */
> + reg = MIPI_DEVICE_READY(pipe);
> + I915_WRITE(reg, val | ULPS_STATE_ENTER);
> + usleep_range(2000, 2500);
> + }
> +
> + /* Enable transparent latch */
> + I915_WRITE(reg, val | LP_OUTPUT_HOLD);
> +
> + if (intel_dsi->dual_link) {
> + count = 2;
> + pipe = PIPE_A;
> + }
> +
> + do {
> + I915_WRITE(reg, val | ULPS_STATE_EXIT);
> + usleep_range(2000, 2500);
> + /* Clear ULPS state */
> + val = I915_READ(MIPI_DEVICE_READY(pipe)) & ~ULPS_STATE_MASK;
> + I915_WRITE(MIPI_DEVICE_READY(pipe), val);
> + usleep_range(2000, 2500);
> +
> + /* Set device ready */
> + val |= DEVICE_READY;
> + I915_WRITE(MIPI_DEVICE_READY(pipe), val);
> + usleep_range(2000, 2500);
> +
> + /* For Port C for dual link */
> + if (intel_dsi->dual_link)
> + pipe = PIPE_B;
> + } while (--count > 0);
You're making functional changes to the BYT/BSW DSI code. These *must*
be separated to independent patches.
Additionally the loop is hard to follow. If you abstract the block into
a separate function, you can just call it once for single link, and
twice for dual link, with appropriate parameters.
> +}
> +
> static void intel_dsi_port_enable(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -294,6 +389,7 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> enum port port;
> u32 temp;
> + u32 port_ctrl;
>
> if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> temp = I915_READ(VLV_CHICKEN_3);
> @@ -304,7 +400,10 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> }
>
> for_each_dsi_port(port, intel_dsi->ports) {
> - temp = I915_READ(MIPI_PORT_CTRL(port));
> + port_ctrl = IS_BROXTON(dev) ? BXT_MIPI_PORT_CTRL(port) :
> + MIPI_PORT_CTRL(port);
> + temp = I915_READ(port_ctrl);
> +
> temp &= ~LANE_CONFIGURATION_MASK;
> temp &= ~DUAL_LINK_MODE_MASK;
>
> @@ -316,8 +415,8 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> LANE_CONFIGURATION_DUAL_LINK_A;
> }
> /* assert ip_tg_enable signal */
> - I915_WRITE(MIPI_PORT_CTRL(port), temp | DPI_ENABLE);
> - POSTING_READ(MIPI_PORT_CTRL(port));
> + I915_WRITE(port_ctrl, temp | DPI_ENABLE);
> + POSTING_READ(port_ctrl);
> }
> }
>
> @@ -339,41 +438,15 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
>
> static void intel_dsi_device_ready(struct intel_encoder *encoder)
> {
> - struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> - struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> - enum port port;
> - u32 val;
> -
> - DRM_DEBUG_KMS("\n");
> -
> - mutex_lock(&dev_priv->dpio_lock);
> - /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
> - * needed everytime after power gate */
> - vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
> - mutex_unlock(&dev_priv->dpio_lock);
> -
> - /* bandgap reset is needed after everytime we do power gate */
> - band_gap_reset(dev_priv);
> -
> - for_each_dsi_port(port, intel_dsi->ports) {
> -
> - I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER);
> - usleep_range(2500, 3000);
> -
> - /* Enable MIPI PHY transparent latch
> - * Common bit for both MIPI Port A & MIPI Port C
> - * No similar bit in MIPI Port C reg
> - */
> - val = I915_READ(MIPI_PORT_CTRL(PORT_A));
> - I915_WRITE(MIPI_PORT_CTRL(PORT_A), val | LP_OUTPUT_HOLD);
> - usleep_range(1000, 1500);
> + struct drm_device *dev = encoder->base.dev;
>
> - I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
> - usleep_range(2500, 3000);
> + if (IS_VALLEYVIEW(dev))
> + vlv_dsi_device_ready(encoder);
> + else if (IS_BROXTON(dev))
> + bxt_dsi_device_ready(encoder);
> + else
> + DRM_ERROR("Invalid DSI device to device ready\n");
Please remove all such else branches.
>
> - I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY);
> - usleep_range(2500, 3000);
> - }
> }
>
> static void intel_dsi_enable(struct intel_encoder *encoder)
> @@ -415,19 +488,22 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>
> DRM_DEBUG_KMS("\n");
>
> - /* Disable DPOunit clock gating, can stall pipe
> - * and we need DPLL REFA always enabled */
> - tmp = I915_READ(DPLL(pipe));
> - tmp |= DPLL_REFA_CLK_ENABLE_VLV;
> - I915_WRITE(DPLL(pipe), tmp);
> + if (IS_VALLEYVIEW(dev)) {
> + /* Disable DPOunit clock gating, can stall pipe
> + * and we need DPLL REFA always enabled
> + */
> + tmp = I915_READ(DPLL(pipe));
> + tmp |= DPLL_REFA_CLK_ENABLE_VLV;
> + I915_WRITE(DPLL(pipe), tmp);
>
> - /* update the hw state for DPLL */
> - intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> - DPLL_REFA_CLK_ENABLE_VLV;
> + /* update the hw state for DPLL */
> + intel_crtc->config->dpll_hw_state.dpll =
> + DPLL_INTEGRATED_CLOCK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
>
> - tmp = I915_READ(DSPCLK_GATE_D);
> - tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> - I915_WRITE(DSPCLK_GATE_D, tmp);
> + tmp = I915_READ(DSPCLK_GATE_D);
> + tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> + I915_WRITE(DSPCLK_GATE_D, tmp);
> + }
>
> /* put device in ready state */
> intel_dsi_device_ready(encoder);
> @@ -959,11 +1035,24 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>
> static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
> {
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> DRM_DEBUG_KMS("\n");
>
> + if (IS_VALLEYVIEW(dev)) {
> + mutex_lock(&dev_priv->dpio_lock);
> + /* program rcomp for compliance, reduce from 50 ohms to 45 ohms
> + * needed everytime after power gate */
> + vlv_flisdsi_write(dev_priv, 0x04, 0x0004);
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> + /* bandgap reset is needed after everytime we do power gate */
> + band_gap_reset(dev_priv);
> + }
> +
You're making functional changes to BYT/BSW DSI code. These *must* be
separated to independent patches.
BR,
Jani.
> intel_dsi_prepare(encoder);
> intel_enable_dsi_pll(encoder);
> -
> }
>
> static enum drm_connector_status
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 08/12] drm/i915/bxt: DSI disable and post-disable
2015-05-22 16:06 ` [PATCH 08/12] drm/i915/bxt: DSI disable and post-disable Uma Shankar
@ 2015-05-25 16:44 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:44 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> This patch contains changes to support DSI disble sequence in BXT.
> The changes are:
> 1. BXT specific changes in clear_device_ready function.
> 2. BXT specific changes in DSI disable and post-disable functions.
> 3. Add a new function to reset BXT Dphy clock and dividers
> (bxt_dsi_reset_clocks).
> 4. Moved some part of the vlv clock reset code, in a new function
> (vlv_dsi_reset_clocks) maintaining the exact same sequence.
> 5. Wrapper function to call corresponding reset clock function.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 39 ++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_dsi.h | 2 ++
> drivers/gpu/drm/i915/intel_dsi_pll.c | 41 ++++++++++++++++++++++++++++++++++
> 3 files changed, 68 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 729faf6..e4b96bc 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -427,12 +427,16 @@ static void intel_dsi_port_disable(struct intel_encoder *encoder)
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> enum port port;
> u32 temp;
> + u32 port_ctrl;
>
> for_each_dsi_port(port, intel_dsi->ports) {
> /* de-assert ip_tg_enable signal */
> - temp = I915_READ(MIPI_PORT_CTRL(port));
> - I915_WRITE(MIPI_PORT_CTRL(port), temp & ~DPI_ENABLE);
> - POSTING_READ(MIPI_PORT_CTRL(port));
> + port_ctrl = IS_BROXTON(dev) ?
> + BXT_MIPI_PORT_CTRL(port) :
> + MIPI_PORT_CTRL(port);
> + temp = I915_READ(port_ctrl);
> + I915_WRITE(port_ctrl, temp & ~DPI_ENABLE);
> + POSTING_READ(port_ctrl);
> }
> }
>
> @@ -570,12 +574,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
> /* Panel commands can be sent when clock is in LP11 */
> I915_WRITE(MIPI_DEVICE_READY(port), 0x0);
>
> - temp = I915_READ(MIPI_CTRL(port));
> - temp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
> - I915_WRITE(MIPI_CTRL(port), temp |
> - intel_dsi->escape_clk_div <<
> - ESCAPE_CLOCK_DIVIDER_SHIFT);
> -
> + intel_dsi_reset_clocks(encoder, port);
> I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);
>
> temp = I915_READ(MIPI_DSI_FUNC_PRG(port));
> @@ -594,12 +593,15 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
>
> static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
> {
> + struct drm_device *dev = encoder->base.dev;
You don't need dev. IS_BROXTON and IS_VALLEYVIEW eat both dev and
dev_priv.
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> enum port port;
> u32 val;
> + u32 port_ctrl;
>
> DRM_DEBUG_KMS("\n");
> +
Unrelated whitespace change.
> for_each_dsi_port(port, intel_dsi->ports) {
>
> I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY |
> @@ -614,18 +616,27 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
> ULPS_STATE_ENTER);
> usleep_range(2000, 2500);
>
> + if (IS_BROXTON(dev))
> + port_ctrl = BXT_MIPI_PORT_CTRL(port);
> + else if (IS_VALLEYVIEW(dev))
> + port_ctrl = MIPI_PORT_CTRL(PORT_A);
> + else {
> + DRM_ERROR("Invalid DSI device to clear device ready\n");
> + return;
Please drop all such else branches.
> + }
> +
> /* Wait till Clock lanes are in LP-00 state for MIPI Port A
> * only. MIPI Port C has no similar bit for checking
> */
> - if (wait_for(((I915_READ(MIPI_PORT_CTRL(PORT_A)) & AFE_LATCHOUT)
> + if (wait_for(((I915_READ(port_ctrl) & AFE_LATCHOUT)
> == 0x00000), 30))
> DRM_ERROR("DSI LP not going Low\n");
>
> /* Disable MIPI PHY transparent latch
> * Common bit for both MIPI Port A & MIPI Port C
> */
> - val = I915_READ(MIPI_PORT_CTRL(PORT_A));
> - I915_WRITE(MIPI_PORT_CTRL(PORT_A), val & ~LP_OUTPUT_HOLD);
> + val = I915_READ(port_ctrl);
> + I915_WRITE(port_ctrl, val & ~LP_OUTPUT_HOLD);
> usleep_range(1000, 1500);
>
> I915_WRITE(MIPI_DEVICE_READY(port), 0x00);
> @@ -637,14 +648,14 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
>
> static void intel_dsi_post_disable(struct intel_encoder *encoder)
> {
> - struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
Unnecessary change.
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> u32 val;
>
> DRM_DEBUG_KMS("\n");
>
> intel_dsi_disable(encoder);
> -
Unrelated whitespace change.
> intel_dsi_clear_device_ready(encoder);
>
> val = I915_READ(DSPCLK_GATE_D);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index af5a09f..8bc8d94 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -125,6 +125,8 @@ extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
> extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
> extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
> extern void bxt_dsi_program_clocks(struct drm_device *dev, int pipe);
> +extern void intel_dsi_reset_clocks(struct intel_encoder *encoder,
> + enum port port);
>
> struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 9a8a35d..49330b0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -369,6 +369,19 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
> return pclk;
> }
>
> +void vlv_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
> +{
> + u32 temp;
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +
> + temp = I915_READ(MIPI_CTRL(port));
> + temp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
> + I915_WRITE(MIPI_CTRL(port), temp |
> + intel_dsi->escape_clk_div <<
> + ESCAPE_CLOCK_DIVIDER_SHIFT);
> +}
> +
> /* Program BXT Mipi clocks and dividers */
> void bxt_dsi_program_clocks(struct drm_device *dev, int pipe)
> {
> @@ -515,3 +528,31 @@ void intel_disable_dsi_pll(struct intel_encoder *encoder)
> else
> DRM_ERROR("Invalid DSI device to pre_pll_enable\n");
> }
> +
> +void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
> +{
> + u32 tmp;
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + /* Clear old configurations */
> + tmp = I915_READ(BXT_MIPI_CLOCK_CTL);
> + tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(port));
> + tmp &= ~(BXT_MIPI_RX_ESCLK_FIXDIV_MASK(port));
> + tmp &= ~(BXT_MIPI_ESCLK_VAR_DIV_MASK(port));
> + tmp &= ~(BXT_MIPI_DPHY_DIVIDER_MASK(port));
> + I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
> + I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP);
> +}
> +
> +void intel_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
> +{
> + struct drm_device *dev = encoder->base.dev;
> +
> + if (IS_BROXTON(dev))
> + bxt_dsi_reset_clocks(encoder, port);
> + else if (IS_VALLEYVIEW(dev))
> + vlv_dsi_reset_clocks(encoder, port);
> + else
> + DRM_ERROR("Invalid DSI device to reset clocks\n");
Please drop all such else branches.
> +}
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks
2015-05-22 16:06 ` [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks Uma Shankar
@ 2015-05-25 16:52 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:52 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> BXT DSI clocks are different than previous platforms. So adding a
> new function to program following clocks and dividers:
> 1. Program variable divider to generate input to Tx clock divider
> (Output value must be < 39.5Mhz)
> 2. Select divide by 2 option to get < 20Mhz for Tx clock
> 3. Program 8by3 divider to generate Rx clock
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 51 ++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_dsi.c | 3 ++
> drivers/gpu/drm/i915/intel_dsi.h | 1 +
> drivers/gpu/drm/i915/intel_dsi_pll.c | 35 +++++++++++++++++++++++
> 4 files changed, 90 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5eeb2b7..f96f049 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7387,6 +7387,57 @@ enum skl_disp_power_wells {
> #define GEN9_FUSE_PG1_ENABLED (1 << 26)
> #define GEN9_FUSE_PG0_ENABLED (1 << 27)
>
> +/* BXT MIPI clock controls */
> +#define BXT_MAX_VAR_OUTPUT_KHZ 39500
> +#define BXT_MIPI_CLOCK_CTL 0x46090
> +#define BXT_MIPI_DIV_SHIFT 16
Same comments here, above and below, as in previous patches about
conventions for defining registers.
> +
> +/* Var clock divider to generate TX source. Result must be < 39.5 M */
> +#define BXT_MIPI1_ESCLK_VAR_DIV_MASK (0x3F << 26)
> +/* BXT MIPI clock controls */
> +#define BXT_MAX_VAR_OUTPUT_KHZ 39500
> +#define BXT_MIPI_CLOCK_CTL 0x46090
> +#define BXT_MIPI_DIV_SHIFT 16
> +
> +/* Var clock divider to generate TX source. Result must be < 39.5 M */
> +#define BXT_MIPI1_ESCLK_VAR_DIV_MASK (0x3F << 26)
> +#define BXT_MIPI2_ESCLK_VAR_DIV_MASK (0x3F << 10)
> +#define BXT_MIPI_ESCLK_VAR_DIV_MASK(check) \
> + (0x3F << ((!check) * BXT_MIPI_DIV_SHIFT + 10))
check?
Ugh, I hate the confusion between pipe and port for DSI. Yes, I'm
confused too.
Whichever it is, I'd prefer not using !check, because it conflates pipe
B and port C, adding to the confusion.
> +#define BXT_MIPI_ESCLK_VAR_DIV(check, val) \
> + (val << ((!check) * BXT_MIPI_DIV_SHIFT + 10))
> +
> +/* TX control divider to select actual TX clock output from (8x/var) */
> +#define BXT_MIPI1_TX_ESCLK_FIXDIV_MASK (3 << 21)
> +#define BXT_MIPI2_TX_ESCLK_FIXDIV_MASK (3 << 5)
> +#define BXT_MIPI_TX_ESCLK_FIXDIV_MASK(check) \
> + (3 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +
> +#define BXT_MIPI_TX_ESCLK_8XDIV_BY2(check) \
> + (0x0 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +#define BXT_MIPI_TX_ESCLK_8XDIV_BY4(check) \
> + (0x1 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +#define BXT_MIPI_TX_ESCLK_8XDIV_BY8(check) \
> + (0x2 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +
> +/* RX control divider to select actual RX clock output from 8x*/
> +#define BXT_MIPI1_RX_ESCLK_FIXDIV_MASK (3 << 19)
> +#define BXT_MIPI2_RX_ESCLK_FIXDIV_MASK (3 << 3)
> +#define BXT_MIPI_RX_ESCLK_FIXDIV_MASK(check) \
> + (3 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +#define BXT_MIPI_RX_ESCLK_8X_BY2(check) \
> + (1 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +#define BXT_MIPI_RX_ESCLK_8X_BY3(check) \
> + (2 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +#define BXT_MIPI_RX_ESCLK_8X_BY4(check) \
> + (3 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +
> +/* BXT: Always prog DPHY dividers to 00 */
> +#define BXT_MIPI_1_DPHY_DIVIDER_MASK (3 << 16)
> +#define BXT_MIPI_2_DPHY_DIVIDER_MASK (3 << 0)
> +#define BXT_MIPI_DPHY_DIVIDER_MASK(check) \
> + (3 << ((!check) * BXT_MIPI_DIV_SHIFT))
> +
> /* BXT MIPI mode configure */
> #define _BXT_MIPIA_TRANS_HACTIVE 0x6B0F8
> #define _BXT_MIPIC_TRANS_HACTIVE 0x6B8F8
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3a1bb04..729faf6 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -503,6 +503,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> tmp = I915_READ(DSPCLK_GATE_D);
> tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> I915_WRITE(DSPCLK_GATE_D, tmp);
> + } else if (IS_BROXTON(dev)) {
> + /* Program Tx Rx and Dphy clocks */
> + bxt_dsi_program_clocks(dev, intel_dsi->ports);
intel_dsi->ports is a bit mask of ports, e.g. (1 << PORT_A) | (1 <<
PORT_B). I don't think this is what you mean or want to use here.
> }
>
> /* put device in ready state */
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 759983e..af5a09f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -124,6 +124,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
> extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
> extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
> extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
> +extern void bxt_dsi_program_clocks(struct drm_device *dev, int pipe);
>
> struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 0cbcf32..9a8a35d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -369,6 +369,41 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
> return pclk;
> }
>
> +/* Program BXT Mipi clocks and dividers */
> +void bxt_dsi_program_clocks(struct drm_device *dev, int pipe)
That should probably be enum pipe or enum port...
> +{
> + u32 tmp = 0;
> + u32 divider = 0;
> + u32 dsi_rate = 0;
> + u32 pll_ratio = 0;
Unnecessary initializations.
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + /* Clear old configurations */
> + tmp = I915_READ(BXT_MIPI_CLOCK_CTL);
> + tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(pipe));
> + tmp &= ~(BXT_MIPI_RX_ESCLK_FIXDIV_MASK(pipe));
> + tmp &= ~(BXT_MIPI_ESCLK_VAR_DIV_MASK(pipe));
> + tmp &= ~(BXT_MIPI_DPHY_DIVIDER_MASK(pipe));
> +
> + /* Get the current DSI rate(actual) */
> + pll_ratio = I915_READ(BXT_DSI_PLL_CTL) &
> + BXT_DSI_MASK_PLL_RATIO;
> + dsi_rate = (BXT_REF_CLOCK_KHZ * pll_ratio) / 2;
> +
> + /* Max possible output of clock is 39.5 MHz, program value -1 */
> + divider = (dsi_rate / BXT_MAX_VAR_OUTPUT_KHZ) - 1;
> + tmp |= BXT_MIPI_ESCLK_VAR_DIV(pipe, divider);
> +
> + /* Tx escape clock should be >=20MHz, so select divide by 2 */
> + tmp |= BXT_MIPI_TX_ESCLK_8XDIV_BY2(pipe);
> +
> + /* Rx escape clock, select fix divide by 3 clock */
> + tmp |= BXT_MIPI_RX_ESCLK_8X_BY3(pipe);
> +
> + /* Do the honors */
> + I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
> +}
> +
> static bool bxt_configure_dsi_pll(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 10/12] drm/i915/bxt: get DSI pixelclock
2015-05-22 16:06 ` [PATCH 10/12] drm/i915/bxt: get DSI pixelclock Uma Shankar
@ 2015-05-25 16:54 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:54 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> BXT's DSI PLL is different from that of VLV. So this patch
> adds a new function to get the current DSI pixel clock based
> on the PLL divider ratio and lane count.
>
> This function is required for intel_dsi_get_config() function.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 10 ++++++++--
> drivers/gpu/drm/i915/intel_dsi.h | 1 +
> drivers/gpu/drm/i915/intel_dsi_pll.c | 35 ++++++++++++++++++++++++++++++++++
> 3 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 2663cdd..5fbcebe 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -714,7 +714,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> static void intel_dsi_get_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config)
> {
> - u32 pclk;
> + u32 pclk = 0;
Unnecessary initialization.
> DRM_DEBUG_KMS("\n");
>
> /*
> @@ -723,7 +723,13 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> */
> pipe_config->dpll_hw_state.dpll_md = 0;
>
> - pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> + if (IS_BROXTON(encoder->base.dev))
> + pclk = bxt_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> + else if (IS_VALLEYVIEW(encoder->base.dev))
> + pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> + else
> + DRM_ERROR("Invalid DSI device to get config\n");
Please drop the else branch.
> +
> if (!pclk)
> return;
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 8bc8d94..01e08e7 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -124,6 +124,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
> extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
> extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
> extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
> +extern u32 bxt_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
> extern void bxt_dsi_program_clocks(struct drm_device *dev, int pipe);
> extern void intel_dsi_reset_clocks(struct intel_encoder *encoder,
> enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 49330b0..073bd1f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -369,6 +369,41 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
> return pclk;
> }
>
> +u32 bxt_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
> +{
> + u32 pclk;
> + u32 dsi_clk;
> + u32 dsi_ratio;
> + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +
> + /* Divide by zero */
> + if (!pipe_bpp) {
> + DRM_ERROR("Invalid BPP(0)\n");
> + return 0;
> + }
> +
> + dsi_ratio = I915_READ(BXT_DSI_PLL_CTL) &
> + BXT_DSI_MASK_PLL_RATIO;
> +
> + /* Invalid DSI ratio ? */
> + if (dsi_ratio < BXT_DSI_PLL_RATIO_MIN ||
> + dsi_ratio > BXT_DSI_PLL_RATIO_MAX) {
> + DRM_ERROR("Invalid DSI pll ratio(%u) programmed\n", dsi_ratio);
> + return 0;
> + }
> +
> + dsi_clk = (dsi_ratio * BXT_REF_CLOCK_KHZ) / 2;
> +
> + /* pixel_format and pipe_bpp should agree */
> + assert_bpp_mismatch(intel_dsi->pixel_format, pipe_bpp);
> +
> + pclk = DIV_ROUND_CLOSEST(dsi_clk * intel_dsi->lane_count, pipe_bpp);
> +
> + DRM_DEBUG_DRIVER("Calculated pclk=%u\n", pclk);
> + return pclk;
> +}
> +
> void vlv_dsi_reset_clocks(struct intel_encoder *encoder, enum port port)
> {
> u32 temp;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes
2015-05-25 10:03 ` Jani Nikula
@ 2015-05-25 16:57 ` Jani Nikula
0 siblings, 0 replies; 28+ messages in thread
From: Jani Nikula @ 2015-05-25 16:57 UTC (permalink / raw)
To: Uma Shankar, intel-gfx; +Cc: shobhit.kumar
On Mon, 25 May 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
>> From: Sunil Kamath <sunil.kamath@intel.com>
>>
>> Latest VBT mentions which set of registers will be used for BLC,
>> as controller number field. Making use of this field in BXT
>> BLC implementation. Also, the registers are used in case control
>> pin indicates display DDI. Adding a check for this.
>> According to Bspec, BLC_PWM_*_2 uses the display utility pin for output.
>> To use backlight 2, enable the utility pin with mode = PWM
>> v2: Jani's review comments
>> addressed
>> - Add a prefix _ to BXT BLC registers definitions.
>> - Add "bxt only" comment for u8 controller
>> - Remove control_pin check for DDI controller
>> - Check for valid controller values
>> - Set pipe bits in UTIL_PIN_CTL
>> - Enable/Disable UTIL_PIN_CTL in enable/disable_backlight()
>> - If BLC 2 is used, read active_low_pwm from UTIL_PIN polarity
>> Satheesh's review comment addressed
>> - If UTIL PIN is already enabled, BIOS would have programmed it. No
>> need to disable and enable again.
>> v3: Jani's review comments
>> - add UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK
>> - Disable UTIL_PIN if controller 1 is used
>> - Mask out UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK before enabling
>> UTIL_PIN
>> - check valid controller value in intel_bios.c
>> - add backlight.util_pin_active_low
>> - disable util pin before enabling
>> v4: Change for BXT-PO branch:
>> Stubbed unwanted definition which was existing before
>> because of DC6 patch.
>> UTIL_PIN_MODE_PWM (0x1b << 24)
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Signed-off-by: Sunil Kamath <sunil.kamath@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 28 +++++++---
>> drivers/gpu/drm/i915/intel_drv.h | 2 +
>> drivers/gpu/drm/i915/intel_panel.c | 100 ++++++++++++++++++++++++++++++------
>> 3 files changed, 106 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index f96f049..5e3958f 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3509,17 +3509,29 @@ enum skl_disp_power_wells {
>> #define UTIL_PIN_CTL 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)
>> +
>> /* BXT backlight register definition. */
>> -#define BXT_BLC_PWM_CTL1 0xC8250
>> +#define _BXT_BLC_PWM_CTL1 0xC8250
>> #define BXT_BLC_PWM_ENABLE (1 << 31)
>> #define BXT_BLC_PWM_POLARITY (1 << 29)
>> -#define BXT_BLC_PWM_FREQ1 0xC8254
>> -#define BXT_BLC_PWM_DUTY1 0xC8258
>> -
>> -#define BXT_BLC_PWM_CTL2 0xC8350
>> -#define BXT_BLC_PWM_FREQ2 0xC8354
>> -#define BXT_BLC_PWM_DUTY2 0xC8358
>> -
>> +#define _BXT_BLC_PWM_FREQ1 0xC8254
>> +#define _BXT_BLC_PWM_DUTY1 0xC8258
>> +
>> +#define _BXT_BLC_PWM_CTL2 0xC8350
>> +#define _BXT_BLC_PWM_FREQ2 0xC8354
>> +#define _BXT_BLC_PWM_DUTY2 0xC8358
>> +
>> +#define BXT_BLC_PWM_CTL(controller) _PIPE(controller, \
>> + _BXT_BLC_PWM_CTL1, _BXT_BLC_PWM_CTL2)
>> +#define BXT_BLC_PWM_FREQ(controller) _PIPE(controller, \
>> + _BXT_BLC_PWM_FREQ1, _BXT_BLC_PWM_FREQ2)
>> +#define BXT_BLC_PWM_DUTY(controller) _PIPE(controller, \
>> + _BXT_BLC_PWM_DUTY1, _BXT_BLC_PWM_DUTY2)
>>
>> #define PCH_GTC_CTL 0xe7000
>> #define PCH_GTC_ENABLE (1 << 31)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 47bc729..ee9eb2b 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -182,7 +182,9 @@ struct intel_panel {
>> bool enabled;
>> bool combination_mode; /* gen 2/4 only */
>> bool active_low_pwm;
>> + bool util_pin_active_low; /* bxt+ */
>> struct backlight_device *device;
>> + u8 controller; /* bxt+ only */
>> } backlight;
>>
>> void (*backlight_power)(struct intel_connector *, bool enable);
>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> index 7d83527..23847f2 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -32,7 +32,10 @@
>>
>> #include <linux/kernel.h>
>> #include <linux/moduleparam.h>
>> +#include <drm/drm_panel.h>
>> #include "intel_drv.h"
>> +#include "intel_dsi.h"
>> +#include "i915_drv.h"
>>
>> void
>> intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>> @@ -539,9 +542,10 @@ static u32 vlv_get_backlight(struct intel_connector *connector)
>> static u32 bxt_get_backlight(struct intel_connector *connector)
>> {
>> struct drm_device *dev = connector->base.dev;
>> + struct intel_panel *panel = &connector->panel;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>>
>> - return I915_READ(BXT_BLC_PWM_DUTY1);
>> + return I915_READ(BXT_BLC_PWM_DUTY(panel->backlight.controller));
>> }
>>
>> static u32 intel_panel_get_backlight(struct intel_connector *connector)
>> @@ -628,8 +632,9 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level)
>> {
>> struct drm_device *dev = connector->base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct intel_panel *panel = &connector->panel;
>>
>> - I915_WRITE(BXT_BLC_PWM_DUTY1, level);
>> + I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller), level);
>> }
>>
>> static void
>> @@ -761,12 +766,20 @@ static void bxt_disable_backlight(struct intel_connector *connector)
>> {
>> struct drm_device *dev = connector->base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> - u32 tmp;
>> + struct intel_panel *panel = &connector->panel;
>> + u32 tmp, val;
>>
>> intel_panel_actually_set_backlight(connector, 0);
>>
>> - tmp = I915_READ(BXT_BLC_PWM_CTL1);
>> - I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE);
>> + tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>> + tmp & ~BXT_BLC_PWM_ENABLE);
>> +
>> + if (panel->backlight.controller == 1) {
>> + val = I915_READ(UTIL_PIN_CTL);
>> + val &= ~UTIL_PIN_ENABLE;
>> + I915_WRITE(UTIL_PIN_CTL, val);
>> + }
>> }
>>
>> void intel_panel_disable_backlight(struct intel_connector *connector)
>> @@ -980,16 +993,39 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>> struct drm_device *dev = connector->base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_panel *panel = &connector->panel;
>> - u32 pwm_ctl;
>> + enum pipe pipe = intel_get_pipe_from_connector(connector);
>> + u32 pwm_ctl, val;
>>
>> - pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
>> + /* To use 2nd set of backlight registers, utility pin has to be
>> + * enabled with PWM mode.
>> + * The field should only be changed when the utility pin is disabled
>> + */
>> + if (panel->backlight.controller == 1) {
>> + val = I915_READ(UTIL_PIN_CTL);
>> + if (val & UTIL_PIN_ENABLE) {
>> + DRM_DEBUG_KMS("util pin already enabled\n");
>> + val &= ~UTIL_PIN_ENABLE;
>> + I915_WRITE(UTIL_PIN_CTL, val);
>> + }
>> + /* mask out UTIL_PIN_PIPE and UTIL_PIN_MODE */
>> + val &= ~(UTIL_PIN_PIPE_MASK | UTIL_PIN_MODE_MASK);
>> + I915_WRITE(UTIL_PIN_CTL, val);
>> + if (panel->backlight.util_pin_active_low)
>> + val |= UTIL_PIN_POLARITY;
>> + I915_WRITE(UTIL_PIN_CTL, val | UTIL_PIN_PIPE(pipe) |
>> + UTIL_PIN_MODE_PWM | UTIL_PIN_ENABLE);
>> + }
>> +
>> + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> if (pwm_ctl & BXT_BLC_PWM_ENABLE) {
>> DRM_DEBUG_KMS("backlight already enabled\n");
>> pwm_ctl &= ~BXT_BLC_PWM_ENABLE;
>> - I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
>> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>> + pwm_ctl);
>> }
>>
>> - I915_WRITE(BXT_BLC_PWM_FREQ1, panel->backlight.max);
>> + I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
>> + panel->backlight.max);
>>
>> intel_panel_actually_set_backlight(connector, panel->backlight.level);
>>
>> @@ -997,9 +1033,10 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>> if (panel->backlight.active_low_pwm)
>> pwm_ctl |= BXT_BLC_PWM_POLARITY;
>>
>> - I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
>> - POSTING_READ(BXT_BLC_PWM_CTL1);
>> - I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE);
>> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), pwm_ctl);
>> + POSTING_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> + I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>> + pwm_ctl | BXT_BLC_PWM_ENABLE);
>> }
>>
>> void intel_panel_enable_backlight(struct intel_connector *connector)
>> @@ -1008,6 +1045,9 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_panel *panel = &connector->panel;
>> enum pipe pipe = intel_get_pipe_from_connector(connector);
>> + struct intel_dsi *intel_dsi = NULL;
>> + struct drm_crtc *crtc = NULL;
>> + struct intel_encoder *encoder = NULL;
>>
>> if (!panel->backlight.present)
>> return;
>> @@ -1027,7 +1067,18 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>> panel->backlight.device->props.max_brightness);
>> }
>>
>> - dev_priv->display.enable_backlight(connector);
>> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>> + for_each_encoder_on_crtc(dev, crtc, encoder) {
>> + if (encoder->type == INTEL_OUTPUT_DSI)
>> + intel_dsi = enc_to_intel_dsi(&encoder->base);
>> + }
>> + }
>> +
>> + if (IS_BROXTON(dev) && intel_dsi->panel->funcs->enable)
>> + intel_dsi->panel->funcs->enable(intel_dsi->panel);
>> + else
>> + dev_priv->display.enable_backlight(connector);
>> +
>
> This is backwards. The DSI code should call intel_panel_enable_backlight
> and intel_panel_disable_backlight, not the other way round.
The code should also call setup backlight. Presumably there won't (at
least initially) be a setup with both eDP and DSI.
BR,
Jani.
>
> BR,
> Jani.
>
>
>> panel->backlight.enabled = true;
>> if (panel->backlight.device)
>> panel->backlight.device->props.power = FB_BLANK_UNBLANK;
>> @@ -1362,10 +1413,27 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
>> struct intel_panel *panel = &connector->panel;
>> u32 pwm_ctl, val;
>>
>> - pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
>> - panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
>> + /* For BXT hard coding the Backlight controller to 0.
>> + * ToDo : Read the controller value from VBT and generalize
>> + */
>> + panel->backlight.controller = 0;
>> +
>> + pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> +
>> + /* Keeping the check if controller 1 is to be programmed.
>> + * This will come into affect once the VBT parsing
>> + * is fixed for controller selection, and controller 1 is used
>> + * for a prticular display configuration.
>> + */
>> + if (panel->backlight.controller == 1) {
>> + val = I915_READ(UTIL_PIN_CTL);
>> + panel->backlight.util_pin_active_low =
>> + val & UTIL_PIN_POLARITY;
>> + }
>>
>> - panel->backlight.max = I915_READ(BXT_BLC_PWM_FREQ1);
>> + panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
>> + panel->backlight.max = I915_READ(
>> + BXT_BLC_PWM_FREQ(panel->backlight.controller));
>> if (!panel->backlight.max)
>> return -ENODEV;
>>
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset
2015-05-25 10:25 ` Jani Nikula
@ 2015-05-26 7:11 ` Daniel Vetter
2015-05-26 7:19 ` Jani Nikula
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Vetter @ 2015-05-26 7:11 UTC (permalink / raw)
To: Jani Nikula; +Cc: shobhit.kumar, intel-gfx
On Mon, May 25, 2015 at 01:25:56PM +0300, Jani Nikula wrote:
> On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> > + * but DDI interface doesn't support DSI yet, so don't do anything
> > + * for DSI encoders
> > + */
> > + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
>
> HAS_DDI() is always true here.
>
> Hmm. Perhaps it would be nicer if we added INVALID_PORT = -1 to enum
> port, and had intel_ddi_get_encoder_port() return that for DSI. Then we
> could leave most of the functions the same, with just
>
> if (port == INVALID_PORT)
> return;
>
> at the beginning.
>
> Daniel, opinions?
Layering in the ddi/hsw+ display code is a bit fumbled - a bunch of these
ddi enable/disable calls should be pushed down into encoder hooks.
Otherwise we need to sprinkle piles of if (type == foo) checks all over.
Well we already have them, but we'd need more :(
Generally the split between crtc and encoder should be at the cross-bar
for most of the ports (pch-split is special here with fdi vs cpu ports).
Especially here where we already have a ddi encoder to handle all the ddi
common code.
I've started with patches a while ago, but that didn't get all that far.
Imo the crucial bit is to get rid of intel_ddi_get_encoder_port is the
indicator for how much layering confusion there still is in the ddi code.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset
2015-05-26 7:11 ` Daniel Vetter
@ 2015-05-26 7:19 ` Jani Nikula
2015-05-26 8:26 ` Daniel Vetter
0 siblings, 1 reply; 28+ messages in thread
From: Jani Nikula @ 2015-05-26 7:19 UTC (permalink / raw)
To: Daniel Vetter; +Cc: shobhit.kumar, intel-gfx
On Tue, 26 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, May 25, 2015 at 01:25:56PM +0300, Jani Nikula wrote:
>> On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
>> > + * but DDI interface doesn't support DSI yet, so don't do anything
>> > + * for DSI encoders
>> > + */
>> > + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
>>
>> HAS_DDI() is always true here.
>>
>> Hmm. Perhaps it would be nicer if we added INVALID_PORT = -1 to enum
>> port, and had intel_ddi_get_encoder_port() return that for DSI. Then we
>> could leave most of the functions the same, with just
>>
>> if (port == INVALID_PORT)
>> return;
>>
>> at the beginning.
>>
>> Daniel, opinions?
>
> Layering in the ddi/hsw+ display code is a bit fumbled - a bunch of these
> ddi enable/disable calls should be pushed down into encoder hooks.
> Otherwise we need to sprinkle piles of if (type == foo) checks all over.
> Well we already have them, but we'd need more :(
>
> Generally the split between crtc and encoder should be at the cross-bar
> for most of the ports (pch-split is special here with fdi vs cpu ports).
> Especially here where we already have a ddi encoder to handle all the ddi
> common code.
>
> I've started with patches a while ago, but that didn't get all that far.
> Imo the crucial bit is to get rid of intel_ddi_get_encoder_port is the
> indicator for how much layering confusion there still is in the ddi code.
I guess the question is, what's the short term plan for DSI?
BR,
Jani.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset
2015-05-26 7:19 ` Jani Nikula
@ 2015-05-26 8:26 ` Daniel Vetter
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Vetter @ 2015-05-26 8:26 UTC (permalink / raw)
To: Jani Nikula; +Cc: shobhit.kumar, intel-gfx
On Tue, May 26, 2015 at 10:19:50AM +0300, Jani Nikula wrote:
> On Tue, 26 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, May 25, 2015 at 01:25:56PM +0300, Jani Nikula wrote:
> >> On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> >> > + * but DDI interface doesn't support DSI yet, so don't do anything
> >> > + * for DSI encoders
> >> > + */
> >> > + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
> >>
> >> HAS_DDI() is always true here.
> >>
> >> Hmm. Perhaps it would be nicer if we added INVALID_PORT = -1 to enum
> >> port, and had intel_ddi_get_encoder_port() return that for DSI. Then we
> >> could leave most of the functions the same, with just
> >>
> >> if (port == INVALID_PORT)
> >> return;
> >>
> >> at the beginning.
> >>
> >> Daniel, opinions?
> >
> > Layering in the ddi/hsw+ display code is a bit fumbled - a bunch of these
> > ddi enable/disable calls should be pushed down into encoder hooks.
> > Otherwise we need to sprinkle piles of if (type == foo) checks all over.
> > Well we already have them, but we'd need more :(
> >
> > Generally the split between crtc and encoder should be at the cross-bar
> > for most of the ports (pch-split is special here with fdi vs cpu ports).
> > Especially here where we already have a ddi encoder to handle all the ddi
> > common code.
> >
> > I've started with patches a while ago, but that didn't get all that far.
> > Imo the crucial bit is to get rid of intel_ddi_get_encoder_port is the
> > indicator for how much layering confusion there still is in the ddi code.
>
> I guess the question is, what's the short term plan for DSI?
Just expressing my unhappiness about how convoluted the ddi code is. It's
full of reverse-lookups of ports and big if (type == foo) ladders, which
makes the entire thing fairly unwielding. And now we add more conditions
to it with the bolted-on dsi ports, which doesn't help. Generally big if
ladders on the type in object-oriented code means the type hierarchy ended
up cut the wrong way. Same with all the inversion around looking up the
port.
I guess we could munge on and hope for a better day. But I'd love for
someone to untangle this ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2015-05-26 8:23 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-22 16:05 [PATCH 00/12] *** MIPI DSI Support for BXT *** Uma Shankar
2015-05-22 16:05 ` [PATCH 01/12] drm/i915/bxt: Initialize MIPI for BXT Uma Shankar
2015-05-22 16:05 ` [PATCH 02/12] drm/i915/bxt: Enable BXT DSI PLL Uma Shankar
2015-05-25 16:10 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 03/12] drm/i915/bxt: Disable DSI PLL for BXT Uma Shankar
2015-05-25 16:12 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 04/12] drm/i915/bxt: DSI prepare changes " Uma Shankar
2015-05-25 16:25 ` Jani Nikula
2015-05-22 16:05 ` [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset Uma Shankar
2015-05-25 10:13 ` Jani Nikula
2015-05-25 11:24 ` Jani Nikula
2015-05-25 10:25 ` Jani Nikula
2015-05-26 7:11 ` Daniel Vetter
2015-05-26 7:19 ` Jani Nikula
2015-05-26 8:26 ` Daniel Vetter
2015-05-22 16:05 ` [PATCH 06/12] drm/i915/bxt: DSI enable for BXT Uma Shankar
2015-05-25 16:39 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks Uma Shankar
2015-05-25 16:52 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 08/12] drm/i915/bxt: DSI disable and post-disable Uma Shankar
2015-05-25 16:44 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 09/12] drm/i915/bxt: get_hw_state for BXT Uma Shankar
2015-05-22 16:06 ` [PATCH 10/12] drm/i915/bxt: get DSI pixelclock Uma Shankar
2015-05-25 16:54 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 11/12] drm/i915/bxt: Modify BXT BLC according to VBT changes Uma Shankar
2015-05-25 10:03 ` Jani Nikula
2015-05-25 16:57 ` Jani Nikula
2015-05-22 16:06 ` [PATCH 12/12] drm/i915/bxt: Remove DSP CLK_GATE programming for BXT Uma Shankar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.