* [PATCH 10/14] drm/i915: Add MIPI_IO WA @ 2017-01-09 9:16 Vidya Srinivas 2017-01-12 11:43 ` Mika Kahola 0 siblings, 1 reply; 15+ messages in thread From: Vidya Srinivas @ 2017-01-09 9:16 UTC (permalink / raw) To: intel-gfx From: Uma Shankar <uma.shankar@intel.com> Enable MIPI IO WA for BXT DSI as per bspec. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 71b978a..b9d7e98 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8301,6 +8301,9 @@ enum { #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR _MMIO(0x138090) +#define MIPIO_RST_CTRL (1 << 2) + #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 a4bda92..9252490 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct intel_encoder *encoder) DRM_DEBUG_KMS("\n"); + /* Add MIPI IO reset programming for modeset */ + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, + val | MIPIO_RST_CTRL); + /* Enable MIPI PHY transparent latch */ for_each_dsi_port(port, intel_dsi->ports) { val = I915_READ(BXT_MIPI_PORT_CTRL(port)); @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder, drm_panel_power_off(intel_dsi->panel); msleep(intel_dsi->panel_off_delay); + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, + val & ~MIPIO_RST_CTRL); + intel_disable_dsi_pll(encoder); /* Panel Disable over CRC PMIC */ -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-09 9:16 [PATCH 10/14] drm/i915: Add MIPI_IO WA Vidya Srinivas @ 2017-01-12 11:43 ` Mika Kahola 2017-01-13 7:55 ` Jani Nikula 2017-01-16 10:01 ` [PATCH 10/14] drm/i915: Add MIPI_IO WA Srinivas, Vidya 0 siblings, 2 replies; 15+ messages in thread From: Mika Kahola @ 2017-01-12 11:43 UTC (permalink / raw) To: Vidya Srinivas, intel-gfx This is definitely needed to pass igt test on bxt 'gem_exec_suspend --run-subtest basic-S3' Tested-by: Mika Kahola <mika.kahola@intel.com> On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > From: Uma Shankar <uma.shankar@intel.com> > > Enable MIPI IO WA for BXT DSI as per bspec. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 71b978a..b9d7e98 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8301,6 +8301,9 @@ enum { > #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR _MMIO(0 > x138090) > +#define MIPIO_RST_CTRL (1 << > 2) > + > #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 a4bda92..9252490 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct > intel_encoder *encoder) > > DRM_DEBUG_KMS("\n"); > > + /* Add MIPI IO reset programming for modeset */ > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > + val | MIPIO_RST_CTRL); > + Should we move this WA to intel_dsi_pre_enable() as the counterpart of this WA is defined intel_dsi_post_disable()? > /* Enable MIPI PHY transparent latch */ > for_each_dsi_port(port, intel_dsi->ports) { > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); > @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct > intel_encoder *encoder, > drm_panel_power_off(intel_dsi->panel); > msleep(intel_dsi->panel_off_delay); > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > + val & ~MIPIO_RST_CTRL); > + > intel_disable_dsi_pll(encoder); > > /* Panel Disable over CRC PMIC */ -- Mika Kahola - Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-12 11:43 ` Mika Kahola @ 2017-01-13 7:55 ` Jani Nikula 2017-01-13 11:18 ` Ander Conselvan De Oliveira 2017-01-13 15:03 ` Imre Deak 2017-01-16 10:01 ` [PATCH 10/14] drm/i915: Add MIPI_IO WA Srinivas, Vidya 1 sibling, 2 replies; 15+ messages in thread From: Jani Nikula @ 2017-01-13 7:55 UTC (permalink / raw) To: mika.kahola, Vidya Srinivas, intel-gfx; +Cc: imre.deak On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > This is definitely needed to pass igt test on bxt > > 'gem_exec_suspend --run-subtest basic-S3' > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: >> From: Uma Shankar <uma.shankar@intel.com> >> >> Enable MIPI IO WA for BXT DSI as per bspec. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 3 +++ >> drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 71b978a..b9d7e98 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -8301,6 +8301,9 @@ enum { >> #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 >> #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, >> _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) >> >> +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR _MMIO(0 >> x138090) Observe that this register is already defined as BXT_P_CR_GT_DISP_PWRON, and already used in intel_dpio_phy.c. It seems to me changing the bits in this register should be hooked at the dpio level. Imre? >> +#define MIPIO_RST_CTRL (1 << >> 2) >> + >> #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 a4bda92..9252490 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct >> intel_encoder *encoder) >> >> DRM_DEBUG_KMS("\n"); >> >> + /* Add MIPI IO reset programming for modeset */ >> + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); >> + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, >> + val | MIPIO_RST_CTRL); >> + > Should we move this WA to intel_dsi_pre_enable() as the counterpart of > this WA is defined intel_dsi_post_disable()? As I said, this should probably be managed in intel_dpio_phy.c. And if not, this is BXT specific, and this hunk runs it on everything else too. BR, Jani. > >> /* Enable MIPI PHY transparent latch */ >> for_each_dsi_port(port, intel_dsi->ports) { >> val = I915_READ(BXT_MIPI_PORT_CTRL(port)); >> @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct >> intel_encoder *encoder, >> drm_panel_power_off(intel_dsi->panel); >> msleep(intel_dsi->panel_off_delay); >> >> + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); >> + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, >> + val & ~MIPIO_RST_CTRL); >> + >> intel_disable_dsi_pll(encoder); >> >> /* Panel Disable over CRC PMIC */ -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-13 7:55 ` Jani Nikula @ 2017-01-13 11:18 ` Ander Conselvan De Oliveira 2017-01-13 15:03 ` Imre Deak 1 sibling, 0 replies; 15+ messages in thread From: Ander Conselvan De Oliveira @ 2017-01-13 11:18 UTC (permalink / raw) To: Jani Nikula, mika.kahola, Vidya Srinivas, intel-gfx; +Cc: imre.deak On Fri, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > > > > This is definitely needed to pass igt test on bxt > > > > 'gem_exec_suspend --run-subtest basic-S3' > > > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > > > > > > From: Uma Shankar <uma.shankar@intel.com> > > > > > > Enable MIPI IO WA for BXT DSI as per bspec. > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > > > 2 files changed, 12 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 71b978a..b9d7e98 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -8301,6 +8301,9 @@ enum { > > > #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR _MMIO(0 > > > x138090) > Observe that this register is already defined as BXT_P_CR_GT_DISP_PWRON, > and already used in intel_dpio_phy.c. It seems to me changing the bits > in this register should be hooked at the dpio level. AFAIK this is an uncore register and not exactly part of DPIO. The DPIO phy bits are power requests that go to the P-Unit, so somewhat similar to power well enabling. The DSI usage seems orthogonal to the DPIO phys, so I don't think it makes a lot of sense to do it in intel_dpio_phy.c. Ander > > Imre? > > > > > > > > > +#define MIPIO_RST_CTRL (1 << > > > 2) > > > + > > > #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 a4bda92..9252490 100644 > > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct > > > intel_encoder *encoder) > > > > > > DRM_DEBUG_KMS("\n"); > > > > > > + /* Add MIPI IO reset programming for modeset */ > > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > + val | MIPIO_RST_CTRL); > > > + > > Should we move this WA to intel_dsi_pre_enable() as the counterpart of > > this WA is defined intel_dsi_post_disable()? > As I said, this should probably be managed in intel_dpio_phy.c. > > And if not, this is BXT specific, and this hunk runs it on everything > else too. > > BR, > Jani. > > > > > > > > > > > > /* Enable MIPI PHY transparent latch */ > > > for_each_dsi_port(port, intel_dsi->ports) { > > > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); > > > @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct > > > intel_encoder *encoder, > > > drm_panel_power_off(intel_dsi->panel); > > > msleep(intel_dsi->panel_off_delay); > > > > > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > + val & ~MIPIO_RST_CTRL); > > > + > > > intel_disable_dsi_pll(encoder); > > > > > > /* Panel Disable over CRC PMIC */ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-13 7:55 ` Jani Nikula 2017-01-13 11:18 ` Ander Conselvan De Oliveira @ 2017-01-13 15:03 ` Imre Deak 2017-01-13 15:32 ` Ville Syrjälä 1 sibling, 1 reply; 15+ messages in thread From: Imre Deak @ 2017-01-13 15:03 UTC (permalink / raw) To: Jani Nikula, mika.kahola, Vidya Srinivas, intel-gfx, Ville Syrjälä, Ander Conselvan de Oliveira Cc: imre.deak On pe, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > > This is definitely needed to pass igt test on bxt > > > > 'gem_exec_suspend --run-subtest basic-S3' > > > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > > > From: Uma Shankar <uma.shankar@intel.com> > > > > > > Enable MIPI IO WA for BXT DSI as per bspec. > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > > > 2 files changed, 12 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 71b978a..b9d7e98 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -8301,6 +8301,9 @@ enum { > > > #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR _MMIO(0 > > > x138090) > > Observe that this register is already defined as BXT_P_CR_GT_DISP_PWRON, > and already used in intel_dpio_phy.c. It seems to me changing the bits > in this register should be hooked at the dpio level. > > Imre? At least the RMW access for this register would need locking against a concurrent access via the DDI encoder enable/disable code? We should also make sure the IO is turned off during booting/resuming if DSI won't be used (and so the DSI disable hook won't be called), see the BSpec "Broxton Sequences to Initialize Display". We could do this by enabling/disabling the IO via the power well code which would solve the locking issue too. This would mean using POWER_DOMAIN_PORT_DSI, or adding a new power domain if diverging from the BSpec sequence is a problem (would be worth checking with HW people, since AFAICS we've been doing this so far). Btw, what about the 0x160020, 0x160054 regs? According to BSpec we need to program these too in the same sequence. --Imre > > > +#define MIPIO_RST_CTRL (1 << > > > 2) > > > + > > > #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 a4bda92..9252490 100644 > > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct > > > intel_encoder *encoder) > > > > > > DRM_DEBUG_KMS("\n"); > > > > > > + /* Add MIPI IO reset programming for modeset */ > > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > + val | MIPIO_RST_CTRL); > > > + > > Should we move this WA to intel_dsi_pre_enable() as the counterpart of > > this WA is defined intel_dsi_post_disable()? > > As I said, this should probably be managed in intel_dpio_phy.c. > > And if not, this is BXT specific, and this hunk runs it on everything > else too. > > BR, > Jani. > > > > > > > /* Enable MIPI PHY transparent latch */ > > > for_each_dsi_port(port, intel_dsi->ports) { > > > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); > > > @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct > > > intel_encoder *encoder, > > > drm_panel_power_off(intel_dsi->panel); > > > msleep(intel_dsi->panel_off_delay); > > > > > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > + val & ~MIPIO_RST_CTRL); > > > + > > > intel_disable_dsi_pll(encoder); > > > > > > /* Panel Disable over CRC PMIC */ > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-13 15:03 ` Imre Deak @ 2017-01-13 15:32 ` Ville Syrjälä 2017-01-16 10:06 ` Srinivas, Vidya 0 siblings, 1 reply; 15+ messages in thread From: Ville Syrjälä @ 2017-01-13 15:32 UTC (permalink / raw) To: Imre Deak Cc: Ander Conselvan de Oliveira, Vidya Srinivas, imre.deak, intel-gfx On Fri, Jan 13, 2017 at 05:03:33PM +0200, Imre Deak wrote: > On pe, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: > > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > > > This is definitely needed to pass igt test on bxt > > > > > > 'gem_exec_suspend --run-subtest basic-S3' > > > > > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > > > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > > > > From: Uma Shankar <uma.shankar@intel.com> > > > > > > > > Enable MIPI IO WA for BXT DSI as per bspec. > > > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > > > > 2 files changed, 12 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > b/drivers/gpu/drm/i915/i915_reg.h > > > > index 71b978a..b9d7e98 100644 > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > @@ -8301,6 +8301,9 @@ enum { > > > > #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 > > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > > > > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR _MMIO(0 > > > > x138090) > > > > Observe that this register is already defined as BXT_P_CR_GT_DISP_PWRON, > > and already used in intel_dpio_phy.c. It seems to me changing the bits > > in this register should be hooked at the dpio level. > > > > Imre? > > At least the RMW access for this register would need locking against a > concurrent access via the DDI encoder enable/disable code? Full modesets should be serialized by connection_mutex, or perhaps by some other thing with async modesets. Although I have a feeling that if we're doing async modeset commits without locks half the driveer is going to be on fire. Not sure what people are doing/have planned there. > > We should also make sure the IO is turned off during booting/resuming > if DSI won't be used (and so the DSI disable hook won't be called), see > the BSpec "Broxton Sequences to Initialize Display". We could do this > by enabling/disabling the IO via the power well code which would solve > the locking issue too. This would mean using POWER_DOMAIN_PORT_DSI, or > adding a new power domain if diverging from the BSpec sequence is a > problem (would be worth checking with HW people, since AFAICS we've > been doing this so far). > > Btw, what about the 0x160020, 0x160054 regs? According to BSpec we need > to program these too in the same sequence. > > --Imre > > > > > +#define MIPIO_RST_CTRL (1 << > > > > 2) > > > > + > > > > #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 a4bda92..9252490 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > > > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct > > > > intel_encoder *encoder) > > > > > > > > DRM_DEBUG_KMS("\n"); > > > > > > > > + /* Add MIPI IO reset programming for modeset */ > > > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > > + val | MIPIO_RST_CTRL); > > > > + > > > Should we move this WA to intel_dsi_pre_enable() as the counterpart of > > > this WA is defined intel_dsi_post_disable()? > > > > As I said, this should probably be managed in intel_dpio_phy.c. > > > > And if not, this is BXT specific, and this hunk runs it on everything > > else too. > > > > BR, > > Jani. > > > > > > > > > > > /* Enable MIPI PHY transparent latch */ > > > > for_each_dsi_port(port, intel_dsi->ports) { > > > > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); > > > > @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct > > > > intel_encoder *encoder, > > > > drm_panel_power_off(intel_dsi->panel); > > > > msleep(intel_dsi->panel_off_delay); > > > > > > > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > > + val & ~MIPIO_RST_CTRL); > > > > + > > > > intel_disable_dsi_pll(encoder); > > > > > > > > /* Panel Disable over CRC PMIC */ > > -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-13 15:32 ` Ville Syrjälä @ 2017-01-16 10:06 ` Srinivas, Vidya 2017-01-18 9:38 ` Jani Nikula 2017-01-18 10:16 ` Imre Deak 0 siblings, 2 replies; 15+ messages in thread From: Srinivas, Vidya @ 2017-01-16 10:06 UTC (permalink / raw) To: Ville Syrjälä, Deak, Imre Cc: imre.deak@linux.intel.com, Conselvan De Oliveira, Ander, intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > Sent: Friday, January 13, 2017 9:02 PM > To: Deak, Imre <imre.deak@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com>; Kahola, Mika > <mika.kahola@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; intel- > gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander > <ander.conselvan.de.oliveira@intel.com>; imre.deak@linux.intel.com > Subject: Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA > > On Fri, Jan 13, 2017 at 05:03:33PM +0200, Imre Deak wrote: > > On pe, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: > > > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > > > > This is definitely needed to pass igt test on bxt > > > > > > > > 'gem_exec_suspend --run-subtest basic-S3' > > > > > > > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > > > > > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > > > > > From: Uma Shankar <uma.shankar@intel.com> > > > > > > > > > > Enable MIPI IO WA for BXT DSI as per bspec. > > > > > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > > > > > 2 files changed, 12 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > b/drivers/gpu/drm/i915/i915_reg.h index 71b978a..b9d7e98 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > @@ -8301,6 +8301,9 @@ enum { > > > > > #define _BXT_MIPIC_PORT_CTRL > 0x6B8C0 > > > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > > > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > > > > > > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR > _MMIO(0 > > > > > x138090) > > > > > > Observe that this register is already defined as > > > BXT_P_CR_GT_DISP_PWRON, and already used in intel_dpio_phy.c. It > > > seems to me changing the bits in this register should be hooked at the > dpio level. > > > > > > Imre? > > > > At least the RMW access for this register would need locking against a > > concurrent access via the DDI encoder enable/disable code? > > Full modesets should be serialized by connection_mutex, or perhaps by > some other thing with async modesets. Although I have a feeling that if > we're doing async modeset commits without locks half the driveer is going to > be on fire. Not sure what people are doing/have planned there. I hope, with the current design, writing to this register from DSI path should be okay and we don't need to take any explicit locks. Is this understanding correct ? > > > > > We should also make sure the IO is turned off during booting/resuming > > if DSI won't be used (and so the DSI disable hook won't be called), > > see the BSpec "Broxton Sequences to Initialize Display". We could do > > this by enabling/disabling the IO via the power well code which would > > solve the locking issue too. This would mean using > > POWER_DOMAIN_PORT_DSI, or adding a new power domain if diverging > from > > the BSpec sequence is a problem (would be worth checking with HW > > people, since AFAICS we've been doing this so far). AFAIK, this will be taken care in BIOS itself if there is no DSI connected. If DSI is connected, this can be controlled in DSI disable/enable sequence. > > > > Btw, what about the 0x160020, 0x160054 regs? According to BSpec we > > need to program these too in the same sequence. Yes, this is needed. We have a patch for this. Will float the same for review. Thanks for the input. > > > > --Imre > > > > > > > +#define MIPIO_RST_CTRL (1 << > > > > > 2) > > > > > + > > > > > #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 a4bda92..9252490 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > > > > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct > > > > > intel_encoder *encoder) > > > > > > > > > > DRM_DEBUG_KMS("\n"); > > > > > > > > > > + /* Add MIPI IO reset programming for modeset */ > > > > > + val = > I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > > > + > I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > > > + val | MIPIO_RST_CTRL); > > > > > + > > > > Should we move this WA to intel_dsi_pre_enable() as the > > > > counterpart of this WA is defined intel_dsi_post_disable()? > > > > > > As I said, this should probably be managed in intel_dpio_phy.c. > > > > > > And if not, this is BXT specific, and this hunk runs it on > > > everything else too. > > > > > > BR, > > > Jani. > > > > > > > > > > > > > > > /* Enable MIPI PHY transparent latch */ > > > > > for_each_dsi_port(port, intel_dsi->ports) { > > > > > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); > > > > > @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct > > > > > intel_encoder *encoder, > > > > > drm_panel_power_off(intel_dsi->panel); > > > > > msleep(intel_dsi->panel_off_delay); > > > > > > > > > > + val = > I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > > > > + > I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > > > > + val & ~MIPIO_RST_CTRL); > > > > > + > > > > > intel_disable_dsi_pll(encoder); > > > > > > > > > > /* Panel Disable over CRC PMIC */ > > > > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-16 10:06 ` Srinivas, Vidya @ 2017-01-18 9:38 ` Jani Nikula 2017-01-19 5:37 ` Srinivas, Vidya 2017-01-18 10:16 ` Imre Deak 1 sibling, 1 reply; 15+ messages in thread From: Jani Nikula @ 2017-01-18 9:38 UTC (permalink / raw) To: Srinivas, Vidya, Ville Syrjälä, Deak, Imre Cc: imre.deak@linux.intel.com, Conselvan De Oliveira, Ander, intel-gfx@lists.freedesktop.org On Mon, 16 Jan 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote: >> -----Original Message----- >> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >> Sent: Friday, January 13, 2017 9:02 PM >> To: Deak, Imre <imre.deak@intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com>; Kahola, Mika >> <mika.kahola@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; intel- >> gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander >> <ander.conselvan.de.oliveira@intel.com>; imre.deak@linux.intel.com >> Subject: Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA >> >> On Fri, Jan 13, 2017 at 05:03:33PM +0200, Imre Deak wrote: >> > On pe, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: >> > > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: >> > > > This is definitely needed to pass igt test on bxt >> > > > >> > > > 'gem_exec_suspend --run-subtest basic-S3' >> > > > >> > > > Tested-by: Mika Kahola <mika.kahola@intel.com> >> > > > >> > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: >> > > > > From: Uma Shankar <uma.shankar@intel.com> >> > > > > >> > > > > Enable MIPI IO WA for BXT DSI as per bspec. >> > > > > >> > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> > > > > --- >> > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ >> > > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ >> > > > > 2 files changed, 12 insertions(+) >> > > > > >> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h >> > > > > b/drivers/gpu/drm/i915/i915_reg.h index 71b978a..b9d7e98 100644 >> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h >> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h >> > > > > @@ -8301,6 +8301,9 @@ enum { >> > > > > #define _BXT_MIPIC_PORT_CTRL >> 0x6B8C0 >> > > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, >> > > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) >> > > > > >> > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR >> _MMIO(0 >> > > > > x138090) >> > > >> > > Observe that this register is already defined as >> > > BXT_P_CR_GT_DISP_PWRON, and already used in intel_dpio_phy.c. It >> > > seems to me changing the bits in this register should be hooked at the >> dpio level. >> > > >> > > Imre? >> > >> > At least the RMW access for this register would need locking against a >> > concurrent access via the DDI encoder enable/disable code? >> >> Full modesets should be serialized by connection_mutex, or perhaps by >> some other thing with async modesets. Although I have a feeling that if >> we're doing async modeset commits without locks half the driveer is going to >> be on fire. Not sure what people are doing/have planned there. > I hope, with the current design, writing to this register from DSI path should > be okay and we don't need to take any explicit locks. Is this understanding > correct ? >> >> > >> > We should also make sure the IO is turned off during booting/resuming >> > if DSI won't be used (and so the DSI disable hook won't be called), >> > see the BSpec "Broxton Sequences to Initialize Display". We could do >> > this by enabling/disabling the IO via the power well code which would >> > solve the locking issue too. This would mean using >> > POWER_DOMAIN_PORT_DSI, or adding a new power domain if diverging >> from >> > the BSpec sequence is a problem (would be worth checking with HW >> > people, since AFAICS we've been doing this so far). > AFAIK, this will be taken care in BIOS itself if there is no DSI connected. > If DSI is connected, this can be controlled in DSI disable/enable sequence. >> > >> > Btw, what about the 0x160020, 0x160054 regs? According to BSpec we >> > need to program these too in the same sequence. > Yes, this is needed. We have a patch for this. Will float the same for review. > Thanks for the input. When you do, please reorder your patch series to have fixes in front. BR, Jani. >> > >> > --Imre >> > >> > > > > +#define MIPIO_RST_CTRL (1 << >> > > > > 2) >> > > > > + >> > > > > #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 a4bda92..9252490 100644 >> > > > > --- a/drivers/gpu/drm/i915/intel_dsi.c >> > > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c >> > > > > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct >> > > > > intel_encoder *encoder) >> > > > > >> > > > > DRM_DEBUG_KMS("\n"); >> > > > > >> > > > > + /* Add MIPI IO reset programming for modeset */ >> > > > > + val = >> I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); >> > > > > + >> I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, >> > > > > + val | MIPIO_RST_CTRL); >> > > > > + >> > > > Should we move this WA to intel_dsi_pre_enable() as the >> > > > counterpart of this WA is defined intel_dsi_post_disable()? >> > > >> > > As I said, this should probably be managed in intel_dpio_phy.c. >> > > >> > > And if not, this is BXT specific, and this hunk runs it on >> > > everything else too. >> > > >> > > BR, >> > > Jani. >> > > >> > > >> > > > >> > > > > /* Enable MIPI PHY transparent latch */ >> > > > > for_each_dsi_port(port, intel_dsi->ports) { >> > > > > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); >> > > > > @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct >> > > > > intel_encoder *encoder, >> > > > > drm_panel_power_off(intel_dsi->panel); >> > > > > msleep(intel_dsi->panel_off_delay); >> > > > > >> > > > > + val = >> I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); >> > > > > + >> I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, >> > > > > + val & ~MIPIO_RST_CTRL); >> > > > > + >> > > > > intel_disable_dsi_pll(encoder); >> > > > > >> > > > > /* Panel Disable over CRC PMIC */ >> > > >> >> -- >> Ville Syrjälä >> Intel OTC -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-18 9:38 ` Jani Nikula @ 2017-01-19 5:37 ` Srinivas, Vidya 0 siblings, 0 replies; 15+ messages in thread From: Srinivas, Vidya @ 2017-01-19 5:37 UTC (permalink / raw) To: Jani Nikula, Ville Syrjälä, Deak, Imre Cc: imre.deak@linux.intel.com, Conselvan De Oliveira, Ander, intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@linux.intel.com] > Sent: Wednesday, January 18, 2017 3:08 PM > To: Srinivas, Vidya <vidya.srinivas@intel.com>; Ville Syrjälä > <ville.syrjala@linux.intel.com>; Deak, Imre <imre.deak@intel.com> > Cc: Kahola, Mika <mika.kahola@intel.com>; intel-gfx@lists.freedesktop.org; > Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>; > imre.deak@linux.intel.com > Subject: RE: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA > > On Mon, 16 Jan 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote: > >> -----Original Message----- > >> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >> Sent: Friday, January 13, 2017 9:02 PM > >> To: Deak, Imre <imre.deak@intel.com> > >> Cc: Jani Nikula <jani.nikula@linux.intel.com>; Kahola, Mika > >> <mika.kahola@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; > >> intel- gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander > >> <ander.conselvan.de.oliveira@intel.com>; imre.deak@linux.intel.com > >> Subject: Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA > >> > >> On Fri, Jan 13, 2017 at 05:03:33PM +0200, Imre Deak wrote: > >> > On pe, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: > >> > > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > >> > > > This is definitely needed to pass igt test on bxt > >> > > > > >> > > > 'gem_exec_suspend --run-subtest basic-S3' > >> > > > > >> > > > Tested-by: Mika Kahola <mika.kahola@intel.com> > >> > > > > >> > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > >> > > > > From: Uma Shankar <uma.shankar@intel.com> > >> > > > > > >> > > > > Enable MIPI IO WA for BXT DSI as per bspec. > >> > > > > > >> > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >> > > > > --- > >> > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > >> > > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > >> > > > > 2 files changed, 12 insertions(+) > >> > > > > > >> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > >> > > > > b/drivers/gpu/drm/i915/i915_reg.h index 71b978a..b9d7e98 > >> > > > > 100644 > >> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > >> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > >> > > > > @@ -8301,6 +8301,9 @@ enum { > >> > > > > #define _BXT_MIPIC_PORT_CTRL > >> 0x6B8C0 > >> > > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > >> > > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > >> > > > > > >> > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR > >> _MMIO(0 > >> > > > > x138090) > >> > > > >> > > Observe that this register is already defined as > >> > > BXT_P_CR_GT_DISP_PWRON, and already used in intel_dpio_phy.c. It > >> > > seems to me changing the bits in this register should be hooked > >> > > at the > >> dpio level. > >> > > > >> > > Imre? > >> > > >> > At least the RMW access for this register would need locking > >> > against a concurrent access via the DDI encoder enable/disable code? > >> > >> Full modesets should be serialized by connection_mutex, or perhaps by > >> some other thing with async modesets. Although I have a feeling that > >> if we're doing async modeset commits without locks half the driveer > >> is going to be on fire. Not sure what people are doing/have planned > there. > > I hope, with the current design, writing to this register from DSI > > path should be okay and we don't need to take any explicit locks. Is > > this understanding correct ? > >> > >> > > >> > We should also make sure the IO is turned off during > >> > booting/resuming if DSI won't be used (and so the DSI disable hook > >> > won't be called), see the BSpec "Broxton Sequences to Initialize > >> > Display". We could do this by enabling/disabling the IO via the > >> > power well code which would solve the locking issue too. This would > >> > mean using POWER_DOMAIN_PORT_DSI, or adding a new power > domain if > >> > diverging > >> from > >> > the BSpec sequence is a problem (would be worth checking with HW > >> > people, since AFAICS we've been doing this so far). > > AFAIK, this will be taken care in BIOS itself if there is no DSI connected. > > If DSI is connected, this can be controlled in DSI disable/enable sequence. > >> > > >> > Btw, what about the 0x160020, 0x160054 regs? According to BSpec we > >> > need to program these too in the same sequence. > > Yes, this is needed. We have a patch for this. Will float the same for review. > > Thanks for the input. > > When you do, please reorder your patch series to have fixes in front. Yeah, we will re-send it separately and rebase other patches on top of it. > > BR, > Jani. > > > >> > > >> > --Imre > >> > > >> > > > > +#define MIPIO_RST_CTRL (1 << > >> > > > > 2) > >> > > > > + > >> > > > > #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 a4bda92..9252490 100644 > >> > > > > --- a/drivers/gpu/drm/i915/intel_dsi.c > >> > > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > >> > > > > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct > >> > > > > intel_encoder *encoder) > >> > > > > > >> > > > > DRM_DEBUG_KMS("\n"); > >> > > > > > >> > > > > + /* Add MIPI IO reset programming for modeset */ > >> > > > > + val = > >> I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > >> > > > > + > >> I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > >> > > > > + val | MIPIO_RST_CTRL); > >> > > > > + > >> > > > Should we move this WA to intel_dsi_pre_enable() as the > >> > > > counterpart of this WA is defined intel_dsi_post_disable()? > >> > > > >> > > As I said, this should probably be managed in intel_dpio_phy.c. > >> > > > >> > > And if not, this is BXT specific, and this hunk runs it on > >> > > everything else too. > >> > > > >> > > BR, > >> > > Jani. > >> > > > >> > > > >> > > > > >> > > > > /* Enable MIPI PHY transparent latch */ > >> > > > > for_each_dsi_port(port, intel_dsi->ports) { > >> > > > > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); > >> > > > > @@ -757,6 +762,10 @@ static void > >> > > > > intel_dsi_post_disable(struct intel_encoder *encoder, > >> > > > > drm_panel_power_off(intel_dsi->panel); > >> > > > > msleep(intel_dsi->panel_off_delay); > >> > > > > > >> > > > > + val = > >> I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > >> > > > > + > >> I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > >> > > > > + val & ~MIPIO_RST_CTRL); > >> > > > > + > >> > > > > intel_disable_dsi_pll(encoder); > >> > > > > > >> > > > > /* Panel Disable over CRC PMIC */ > >> > > > >> > >> -- > >> Ville Syrjälä > >> Intel OTC > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-16 10:06 ` Srinivas, Vidya 2017-01-18 9:38 ` Jani Nikula @ 2017-01-18 10:16 ` Imre Deak 2017-01-19 5:36 ` Srinivas, Vidya 2017-01-19 6:11 ` [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators Vidya Srinivas 1 sibling, 2 replies; 15+ messages in thread From: Imre Deak @ 2017-01-18 10:16 UTC (permalink / raw) To: Srinivas, Vidya Cc: Conselvan De Oliveira, Ander, imre.deak@linux.intel.com, intel-gfx@lists.freedesktop.org On Mon, Jan 16, 2017 at 12:06:56PM +0200, Srinivas, Vidya wrote: > > > > -----Original Message----- > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > Sent: Friday, January 13, 2017 9:02 PM > > To: Deak, Imre <imre.deak@intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>; Kahola, Mika > > <mika.kahola@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; intel- > > gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander > > <ander.conselvan.de.oliveira@intel.com>; imre.deak@linux.intel.com > > Subject: Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA > > > > On Fri, Jan 13, 2017 at 05:03:33PM +0200, Imre Deak wrote: > > > On pe, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: > > > > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > > > > > This is definitely needed to pass igt test on bxt > > > > > > > > > > 'gem_exec_suspend --run-subtest basic-S3' > > > > > > > > > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > > > > > > > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > > > > > > From: Uma Shankar <uma.shankar@intel.com> > > > > > > > > > > > > Enable MIPI IO WA for BXT DSI as per bspec. > > > > > > > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > > > > > > 2 files changed, 12 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > > b/drivers/gpu/drm/i915/i915_reg.h index 71b978a..b9d7e98 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > > @@ -8301,6 +8301,9 @@ enum { > > > > > > #define _BXT_MIPIC_PORT_CTRL > > 0x6B8C0 > > > > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > > > > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > > > > > > > > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR > > _MMIO(0 > > > > > > x138090) > > > > > > > > Observe that this register is already defined as > > > > BXT_P_CR_GT_DISP_PWRON, and already used in intel_dpio_phy.c. It > > > > seems to me changing the bits in this register should be hooked at the > > dpio level. > > > > > > > > Imre? > > > > > > At least the RMW access for this register would need locking against a > > > concurrent access via the DDI encoder enable/disable code? > > > > Full modesets should be serialized by connection_mutex, or perhaps by > > some other thing with async modesets. Although I have a feeling that if > > we're doing async modeset commits without locks half the driveer is going to > > be on fire. Not sure what people are doing/have planned there. Ok. I assume the async case will need a generic solution then. > I hope, with the current design, writing to this register from DSI path should > be okay and we don't need to take any explicit locks. Is this understanding > correct ? Yes, based on the above the connection_mutex provides for the serialization. Just use the existing define as Jani said. > > > > > > We should also make sure the IO is turned off during booting/resuming > > > if DSI won't be used (and so the DSI disable hook won't be called), > > > see the BSpec "Broxton Sequences to Initialize Display". We could do > > > this by enabling/disabling the IO via the power well code which would > > > solve the locking issue too. This would mean using > > > POWER_DOMAIN_PORT_DSI, or adding a new power domain if diverging > > from > > > the BSpec sequence is a problem (would be worth checking with HW > > > people, since AFAICS we've been doing this so far). > AFAIK, this will be taken care in BIOS itself if there is no DSI connected. > If DSI is connected, this can be controlled in DSI disable/enable sequence. Yes ideally, but checking at least if this is really the case would make sense. But I'm ok with the current approach for now. > > > > > > Btw, what about the 0x160020, 0x160054 regs? According to BSpec we > > > need to program these too in the same sequence. > Yes, this is needed. We have a patch for this. Will float the same for review. > Thanks for the input. Any reason for having separate patches? --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-18 10:16 ` Imre Deak @ 2017-01-19 5:36 ` Srinivas, Vidya 2017-01-19 6:11 ` [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators Vidya Srinivas 1 sibling, 0 replies; 15+ messages in thread From: Srinivas, Vidya @ 2017-01-19 5:36 UTC (permalink / raw) To: Deak, Imre Cc: Conselvan De Oliveira, Ander, imre.deak@linux.intel.com, intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Deak, Imre > Sent: Wednesday, January 18, 2017 3:46 PM > To: Srinivas, Vidya <vidya.srinivas@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Jani Nikula > <jani.nikula@linux.intel.com>; Kahola, Mika <mika.kahola@intel.com>; intel- > gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander > <ander.conselvan.de.oliveira@intel.com>; imre.deak@linux.intel.com > Subject: Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA > > On Mon, Jan 16, 2017 at 12:06:56PM +0200, Srinivas, Vidya wrote: > > > > > > > -----Original Message----- > > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > > > Sent: Friday, January 13, 2017 9:02 PM > > > To: Deak, Imre <imre.deak@intel.com> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>; Kahola, Mika > > > <mika.kahola@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; > > > intel- gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander > > > <ander.conselvan.de.oliveira@intel.com>; imre.deak@linux.intel.com > > > Subject: Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA > > > > > > On Fri, Jan 13, 2017 at 05:03:33PM +0200, Imre Deak wrote: > > > > On pe, 2017-01-13 at 09:55 +0200, Jani Nikula wrote: > > > > > On Thu, 12 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > > > > > > This is definitely needed to pass igt test on bxt > > > > > > > > > > > > 'gem_exec_suspend --run-subtest basic-S3' > > > > > > > > > > > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > > > > > > > > > > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > > > > > > > From: Uma Shankar <uma.shankar@intel.com> > > > > > > > > > > > > > > Enable MIPI IO WA for BXT DSI as per bspec. > > > > > > > > > > > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > > > > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > > > > > > > 2 files changed, 12 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > > > b/drivers/gpu/drm/i915/i915_reg.h index 71b978a..b9d7e98 > > > > > > > 100644 > > > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > > > @@ -8301,6 +8301,9 @@ enum { > > > > > > > #define _BXT_MIPIC_PORT_CTRL > > > 0x6B8C0 > > > > > > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > > > > > > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > > > > > > > > > > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR > > > _MMIO(0 > > > > > > > x138090) > > > > > > > > > > Observe that this register is already defined as > > > > > BXT_P_CR_GT_DISP_PWRON, and already used in intel_dpio_phy.c. > It > > > > > seems to me changing the bits in this register should be hooked > > > > > at the > > > dpio level. > > > > > > > > > > Imre? > > > > > > > > At least the RMW access for this register would need locking > > > > against a concurrent access via the DDI encoder enable/disable code? > > > > > > Full modesets should be serialized by connection_mutex, or perhaps > > > by some other thing with async modesets. Although I have a feeling > > > that if we're doing async modeset commits without locks half the > > > driveer is going to be on fire. Not sure what people are doing/have > planned there. > > Ok. I assume the async case will need a generic solution then. > > > I hope, with the current design, writing to this register from DSI > > path should be okay and we don't need to take any explicit locks. Is > > this understanding correct ? > > Yes, based on the above the connection_mutex provides for the > serialization. Just use the existing define as Jani said. > > > > > > > > > We should also make sure the IO is turned off during > > > > booting/resuming if DSI won't be used (and so the DSI disable hook > > > > won't be called), see the BSpec "Broxton Sequences to Initialize > > > > Display". We could do this by enabling/disabling the IO via the > > > > power well code which would solve the locking issue too. This > > > > would mean using POWER_DOMAIN_PORT_DSI, or adding a new > power > > > > domain if diverging > > > from > > > > the BSpec sequence is a problem (would be worth checking with HW > > > > people, since AFAICS we've been doing this so far). > > AFAIK, this will be taken care in BIOS itself if there is no DSI connected. > > If DSI is connected, this can be controlled in DSI disable/enable sequence. > > Yes ideally, but checking at least if this is really the case would make sense. > But I'm ok with the current approach for now. > > > > > > > > > Btw, what about the 0x160020, 0x160054 regs? According to BSpec we > > > > need to program these too in the same sequence. > > Yes, this is needed. We have a patch for this. Will float the same for review. > > Thanks for the input. > > Any reason for having separate patches? We can merge them together. Will re-send merging them together. > > --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators 2017-01-18 10:16 ` Imre Deak 2017-01-19 5:36 ` Srinivas, Vidya @ 2017-01-19 6:11 ` Vidya Srinivas 2017-01-19 8:42 ` Mika Kahola 1 sibling, 1 reply; 15+ messages in thread From: Vidya Srinivas @ 2017-01-19 6:11 UTC (permalink / raw) To: intel-gfx; +Cc: jani.nikula, ville.syrjala From: Uma Shankar <uma.shankar@intel.com> Enable MIPI IO WA for BXT DSI as per bspec and program the DSI regulators. v2: Moved IO enable to pre-enable as per Mika's review comments. Also reused the existing register definition for BXT_P_CR_GT_DISP_PWRON. v3: Added Programming the DSI regulators as per disable/enable sequences. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 7 +++++++ drivers/gpu/drm/i915/intel_dsi.c | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 00970aa..0a9ad44 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1553,6 +1553,7 @@ enum skl_disp_power_wells { _MMIO(_BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1)) #define BXT_P_CR_GT_DISP_PWRON _MMIO(0x138090) +#define MIPIO_RST_CTRL (1 << 2) #define _BXT_PHY_CTL_DDI_A 0x64C00 #define _BXT_PHY_CTL_DDI_B 0x64C10 @@ -8301,6 +8302,12 @@ enum { #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) +#define BXT_P_DSI_REGULATOR_CFG _MMIO(0x160020) +#define STAP_SELECT (1 << 0) + +#define BXT_P_DSI_REGULATOR_TX_CTRL _MMIO(0x160054) +#define HS_IO_CTRL_SELECT (1 << 0) + #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 16732e7..043441e 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -548,6 +548,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder, struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); enum port port; + u32 val; DRM_DEBUG_KMS("\n"); @@ -558,6 +559,11 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder, intel_disable_dsi_pll(encoder); intel_enable_dsi_pll(encoder, pipe_config); + /* Add MIPI IO reset programming for modeset */ + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, + val | MIPIO_RST_CTRL); + intel_dsi_prepare(encoder, pipe_config); /* Panel Enable over CRC PMIC */ @@ -575,6 +581,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder, I915_WRITE(DSPCLK_GATE_D, val); } + /* Power up DSI regulator */ + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT); + val = I915_READ(BXT_P_DSI_REGULATOR_TX_CTRL); + val &= ~HS_IO_CTRL_SELECT; + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, val); + /* put device in ready state */ intel_dsi_device_ready(encoder); @@ -707,6 +719,7 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder, { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); + u32 val; DRM_DEBUG_KMS("\n"); @@ -714,8 +727,16 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder, intel_dsi_clear_device_ready(encoder); + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, + val & ~MIPIO_RST_CTRL); + intel_disable_dsi_pll(encoder); + /* Power down DSI regulator to save power */ + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT); + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, HS_IO_CTRL_SELECT); + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { u32 val; -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators 2017-01-19 6:11 ` [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators Vidya Srinivas @ 2017-01-19 8:42 ` Mika Kahola 2017-01-19 9:28 ` Jani Nikula 0 siblings, 1 reply; 15+ messages in thread From: Mika Kahola @ 2017-01-19 8:42 UTC (permalink / raw) To: Vidya Srinivas, intel-gfx; +Cc: jani.nikula, ville.syrjala On Thu, 2017-01-19 at 11:41 +0530, Vidya Srinivas wrote: > From: Uma Shankar <uma.shankar@intel.com> > > Enable MIPI IO WA for BXT DSI as per bspec and > program the DSI regulators. > > v2: Moved IO enable to pre-enable as per Mika's > review comments. Also reused the existing register > definition for BXT_P_CR_GT_DISP_PWRON. > > v3: Added Programming the DSI regulators as per disable/enable > sequences. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 7 +++++++ > drivers/gpu/drm/i915/intel_dsi.c | 21 +++++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 00970aa..0a9ad44 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1553,6 +1553,7 @@ enum skl_disp_power_wells { > _MMIO(_BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1)) > > #define BXT_P_CR_GT_DISP_PWRON _MMIO(0x138090) > +#define MIPIO_RST_CTRL (1 << 2) > > #define _BXT_PHY_CTL_DDI_A 0x64C00 > #define _BXT_PHY_CTL_DDI_B 0x64C10 > @@ -8301,6 +8302,12 @@ enum { > #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > +#define BXT_P_DSI_REGULATOR_CFG _MMIO(0x16002 > 0) > +#define STAP_SELECT (1 << 0) > + > +#define BXT_P_DSI_REGULATOR_TX_CTRL _MMIO(0x160054) > +#define HS_IO_CTRL_SELECT (1 << 0) > + > #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 16732e7..043441e 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -548,6 +548,7 @@ static void intel_dsi_pre_enable(struct > intel_encoder *encoder, > struct drm_i915_private *dev_priv = to_i915(encoder- > >base.dev); > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- > >base); > enum port port; > + u32 val; > > DRM_DEBUG_KMS("\n"); > > @@ -558,6 +559,11 @@ static void intel_dsi_pre_enable(struct > intel_encoder *encoder, > intel_disable_dsi_pll(encoder); > intel_enable_dsi_pll(encoder, pipe_config); > > + /* Add MIPI IO reset programming for modeset */ > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, > + val | MIPIO_RST_CTRL); > + Looking good but overall we still need to address Jani's comment to limit these changes only to Broxton platform. > intel_dsi_prepare(encoder, pipe_config); > > /* Panel Enable over CRC PMIC */ > @@ -575,6 +581,12 @@ static void intel_dsi_pre_enable(struct > intel_encoder *encoder, > I915_WRITE(DSPCLK_GATE_D, val); > } > > + /* Power up DSI regulator */ > + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT); > + val = I915_READ(BXT_P_DSI_REGULATOR_TX_CTRL); > + val &= ~HS_IO_CTRL_SELECT; > + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, val); > + /* put device in ready state */ > intel_dsi_device_ready(encoder); > > @@ -707,6 +719,7 @@ static void intel_dsi_post_disable(struct > intel_encoder *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder- > >base.dev); > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- > >base); > + u32 val; > > DRM_DEBUG_KMS("\n"); > > @@ -714,8 +727,16 @@ static void intel_dsi_post_disable(struct > intel_encoder *encoder, > > intel_dsi_clear_device_ready(encoder); > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, > + val & ~MIPIO_RST_CTRL); > + > intel_disable_dsi_pll(encoder); > > + /* Power down DSI regulator to save power */ > + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT); > + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, HS_IO_CTRL_SELECT); > + > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > u32 val; > -- Mika Kahola - Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators 2017-01-19 8:42 ` Mika Kahola @ 2017-01-19 9:28 ` Jani Nikula 0 siblings, 0 replies; 15+ messages in thread From: Jani Nikula @ 2017-01-19 9:28 UTC (permalink / raw) To: mika.kahola, Vidya Srinivas, intel-gfx; +Cc: ville.syrjala On Thu, 19 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote: > On Thu, 2017-01-19 at 11:41 +0530, Vidya Srinivas wrote: >> From: Uma Shankar <uma.shankar@intel.com> >> >> Enable MIPI IO WA for BXT DSI as per bspec and >> program the DSI regulators. >> >> v2: Moved IO enable to pre-enable as per Mika's >> review comments. Also reused the existing register >> definition for BXT_P_CR_GT_DISP_PWRON. >> >> v3: Added Programming the DSI regulators as per disable/enable >> sequences. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 7 +++++++ >> drivers/gpu/drm/i915/intel_dsi.c | 21 +++++++++++++++++++++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 00970aa..0a9ad44 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -1553,6 +1553,7 @@ enum skl_disp_power_wells { >> _MMIO(_BXT_PHY_CH(phy, ch, reg_ch0, reg_ch1)) >> >> #define BXT_P_CR_GT_DISP_PWRON _MMIO(0x138090) >> +#define MIPIO_RST_CTRL (1 << 2) >> >> #define _BXT_PHY_CTL_DDI_A 0x64C00 >> #define _BXT_PHY_CTL_DDI_B 0x64C10 >> @@ -8301,6 +8302,12 @@ enum { >> #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 >> #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, >> _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) >> >> +#define BXT_P_DSI_REGULATOR_CFG _MMIO(0x16002 >> 0) >> +#define STAP_SELECT (1 << 0) >> + >> +#define BXT_P_DSI_REGULATOR_TX_CTRL _MMIO(0x160054) >> +#define HS_IO_CTRL_SELECT (1 << 0) >> + >> #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 16732e7..043441e 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -548,6 +548,7 @@ static void intel_dsi_pre_enable(struct >> intel_encoder *encoder, >> struct drm_i915_private *dev_priv = to_i915(encoder- >> >base.dev); >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- >> >base); >> enum port port; >> + u32 val; >> >> DRM_DEBUG_KMS("\n"); >> >> @@ -558,6 +559,11 @@ static void intel_dsi_pre_enable(struct >> intel_encoder *encoder, >> intel_disable_dsi_pll(encoder); >> intel_enable_dsi_pll(encoder, pipe_config); >> >> + /* Add MIPI IO reset programming for modeset */ >> + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); >> + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, >> + val | MIPIO_RST_CTRL); >> + > Looking good but overall we still need to address Jani's comment to > limit these changes only to Broxton platform. Please don't send this in reply to the current thread... BR, Jani. > >> intel_dsi_prepare(encoder, pipe_config); >> >> /* Panel Enable over CRC PMIC */ >> @@ -575,6 +581,12 @@ static void intel_dsi_pre_enable(struct >> intel_encoder *encoder, >> I915_WRITE(DSPCLK_GATE_D, val); >> } >> >> + /* Power up DSI regulator */ >> + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT); >> + val = I915_READ(BXT_P_DSI_REGULATOR_TX_CTRL); >> + val &= ~HS_IO_CTRL_SELECT; >> + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, val); >> + /* put device in ready state */ >> intel_dsi_device_ready(encoder); >> >> @@ -707,6 +719,7 @@ static void intel_dsi_post_disable(struct >> intel_encoder *encoder, >> { >> struct drm_i915_private *dev_priv = to_i915(encoder- >> >base.dev); >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder- >> >base); >> + u32 val; >> >> DRM_DEBUG_KMS("\n"); >> >> @@ -714,8 +727,16 @@ static void intel_dsi_post_disable(struct >> intel_encoder *encoder, >> >> intel_dsi_clear_device_ready(encoder); >> >> + val = I915_READ(BXT_P_CR_GT_DISP_PWRON); >> + I915_WRITE(BXT_P_CR_GT_DISP_PWRON, >> + val & ~MIPIO_RST_CTRL); >> + >> intel_disable_dsi_pll(encoder); >> >> + /* Power down DSI regulator to save power */ >> + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT); >> + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, HS_IO_CTRL_SELECT); >> + >> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { >> u32 val; >> -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 10/14] drm/i915: Add MIPI_IO WA 2017-01-12 11:43 ` Mika Kahola 2017-01-13 7:55 ` Jani Nikula @ 2017-01-16 10:01 ` Srinivas, Vidya 1 sibling, 0 replies; 15+ messages in thread From: Srinivas, Vidya @ 2017-01-16 10:01 UTC (permalink / raw) To: Kahola, Mika, intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Kahola, Mika > Sent: Thursday, January 12, 2017 5:13 PM > To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA > > This is definitely needed to pass igt test on bxt > > 'gem_exec_suspend --run-subtest basic-S3' > > Tested-by: Mika Kahola <mika.kahola@intel.com> > > On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote: > > From: Uma Shankar <uma.shankar@intel.com> > > > > Enable MIPI IO WA for BXT DSI as per bspec. > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > drivers/gpu/drm/i915/intel_dsi.c | 9 +++++++++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index 71b978a..b9d7e98 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -8301,6 +8301,9 @@ enum { > > #define _BXT_MIPIC_PORT_CTRL 0x6B8C0 > > #define BXT_MIPI_PORT_CTRL(tc) _MMIO_MIPI(tc, > > _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL) > > > > +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR > _MMIO(0 > > x138090) > > +#define MIPIO_RST_CTRL (1 << > > 2) > > + > > #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 a4bda92..9252490 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct > > intel_encoder *encoder) > > > > DRM_DEBUG_KMS("\n"); > > > > + /* Add MIPI IO reset programming for modeset */ > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > + val | MIPIO_RST_CTRL); > > + > Should we move this WA to intel_dsi_pre_enable() as the counterpart of this > WA is defined intel_dsi_post_disable()? Yes, this can be done in intel_dsi_pre_enable itself. Will make the change and re-send. Thanks for the input. > > > /* Enable MIPI PHY transparent latch */ > > for_each_dsi_port(port, intel_dsi->ports) { > > val = I915_READ(BXT_MIPI_PORT_CTRL(port)); > > @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct > > intel_encoder *encoder, > > drm_panel_power_off(intel_dsi->panel); > > msleep(intel_dsi->panel_off_delay); > > > > + val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR); > > + I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR, > > + val & ~MIPIO_RST_CTRL); > > + > > intel_disable_dsi_pll(encoder); > > > > /* Panel Disable over CRC PMIC */ > -- > Mika Kahola - Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-01-19 9:28 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-09 9:16 [PATCH 10/14] drm/i915: Add MIPI_IO WA Vidya Srinivas 2017-01-12 11:43 ` Mika Kahola 2017-01-13 7:55 ` Jani Nikula 2017-01-13 11:18 ` Ander Conselvan De Oliveira 2017-01-13 15:03 ` Imre Deak 2017-01-13 15:32 ` Ville Syrjälä 2017-01-16 10:06 ` Srinivas, Vidya 2017-01-18 9:38 ` Jani Nikula 2017-01-19 5:37 ` Srinivas, Vidya 2017-01-18 10:16 ` Imre Deak 2017-01-19 5:36 ` Srinivas, Vidya 2017-01-19 6:11 ` [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators Vidya Srinivas 2017-01-19 8:42 ` Mika Kahola 2017-01-19 9:28 ` Jani Nikula 2017-01-16 10:01 ` [PATCH 10/14] drm/i915: Add MIPI_IO WA Srinivas, Vidya
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.