* [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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ messages in thread
From: Srinivas, Vidya @ 2017-01-16 10:06 UTC (permalink / raw)
To: Ville Syrjälä, Deak, Imre
Cc: imre.deak, Conselvan De Oliveira, Ander, intel-gfx
> -----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] 22+ 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; 22+ 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, Conselvan De Oliveira, Ander, intel-gfx
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] 22+ 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; 22+ 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, Conselvan De Oliveira, Ander, intel-gfx
> -----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] 22+ 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; 22+ messages in thread
From: Imre Deak @ 2017-01-18 10:16 UTC (permalink / raw)
To: Srinivas, Vidya; +Cc: Conselvan De Oliveira, Ander, imre.deak, intel-gfx
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] 22+ 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; 22+ messages in thread
From: Srinivas, Vidya @ 2017-01-19 5:36 UTC (permalink / raw)
To: Deak, Imre; +Cc: Conselvan De Oliveira, Ander, imre.deak, intel-gfx
> -----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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ messages in thread
From: Srinivas, Vidya @ 2017-01-16 10:01 UTC (permalink / raw)
To: Kahola, Mika, intel-gfx
> -----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] 22+ messages in thread
* [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
@ 2017-01-19 10:45 Vidya Srinivas
2017-01-19 11:04 ` Jani Nikula
0 siblings, 1 reply; 22+ messages in thread
From: Vidya Srinivas @ 2017-01-19 10:45 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.
v4: Restricting regulator changes to BXT as suggested by
Jani/Mika
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
drivers/gpu/drm/i915/intel_dsi.c | 25 +++++++++++++++++++++++++
2 files changed, 32 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..4dc1293 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,14 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
I915_WRITE(DSPCLK_GATE_D, val);
}
+ /* Power up DSI regulator */
+ if (IS_BROXTON(dev_priv)) {
+ 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 +721,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 +729,18 @@ 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);
+ if (IS_BROXTON(dev_priv)) {
+ /* 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] 22+ messages in thread* Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
2017-01-19 10:45 [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators Vidya Srinivas
@ 2017-01-19 11:04 ` Jani Nikula
2017-01-25 13:48 ` Shankar, Uma
0 siblings, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2017-01-19 11:04 UTC (permalink / raw)
To: Vidya Srinivas, intel-gfx; +Cc: ville.syrjala
On Thu, 19 Jan 2017, Vidya Srinivas <vidya.srinivas@intel.com> 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.
>
> v4: Restricting regulator changes to BXT as suggested by
> Jani/Mika
This applies to BXT_P_CR_GT_DISP_PWRON changes as well.
One other question inline.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
> drivers/gpu/drm/i915/intel_dsi.c | 25 +++++++++++++++++++++++++
> 2 files changed, 32 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..4dc1293 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,14 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
> I915_WRITE(DSPCLK_GATE_D, val);
> }
>
> + /* Power up DSI regulator */
> + if (IS_BROXTON(dev_priv)) {
> + 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);
Why does this specific change warrant a read-modify-write when the other
regulator changes in this patch do a full register write?
Also, the enable and disable sequences seem a bit asymmetric with these
changes, i.e. you enable and disable things in different steps of the
sequences. That's a bit surprising.
(These might have an answer in bspec, but I don't seem to be able to
access that right now.)
BR,
Jani.
> + }
> +
> /* put device in ready state */
> intel_dsi_device_ready(encoder);
>
> @@ -707,6 +721,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 +729,18 @@ 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);
>
> + if (IS_BROXTON(dev_priv)) {
> + /* 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] 22+ messages in thread* Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
2017-01-19 11:04 ` Jani Nikula
@ 2017-01-25 13:48 ` Shankar, Uma
2017-01-25 14:13 ` Vidya Srinivas
0 siblings, 1 reply; 22+ messages in thread
From: Shankar, Uma @ 2017-01-25 13:48 UTC (permalink / raw)
To: Nikula, Jani, Srinivas, Vidya, intel-gfx; +Cc: Syrjala, Ville
>-----Original Message-----
>From: Nikula, Jani
>Sent: Thursday, January 19, 2017 4:34 PM
>To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>gfx@lists.freedesktop.org
>Cc: Shankar, Uma <uma.shankar@intel.com>; Syrjala, Ville
><ville.syrjala@intel.com>
>Subject: Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
>
>On Thu, 19 Jan 2017, Vidya Srinivas <vidya.srinivas@intel.com> 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.
>>
>> v4: Restricting regulator changes to BXT as suggested by Jani/Mika
>
>This applies to BXT_P_CR_GT_DISP_PWRON changes as well.
Yes, this should be under IS_BXT.
>
>One other question inline.
>
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
>> drivers/gpu/drm/i915/intel_dsi.c | 25 +++++++++++++++++++++++++
>> 2 files changed, 32 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..4dc1293 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,14 @@ static void intel_dsi_pre_enable(struct
>intel_encoder *encoder,
>> I915_WRITE(DSPCLK_GATE_D, val);
>> }
>>
>> + /* Power up DSI regulator */
>> + if (IS_BROXTON(dev_priv)) {
>> + 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);
>
>Why does this specific change warrant a read-modify-write when the other
>regulator changes in this patch do a full register write?
Checked this in bspec and looks like we can avoid a read/modify operation.
>
>Also, the enable and disable sequences seem a bit asymmetric with these
>changes, i.e. you enable and disable things in different steps of the
>sequences. That's a bit surprising.
Yes, will update this to maintain the symmetry and re-send.
Thanks Jani for all your valuable inputs.
>
>(These might have an answer in bspec, but I don't seem to be able to access
>that right now.)
>
>BR,
>Jani.
>
>> + }
>> +
>> /* put device in ready state */
>> intel_dsi_device_ready(encoder);
>>
>> @@ -707,6 +721,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 +729,18 @@ 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);
>>
>> + if (IS_BROXTON(dev_priv)) {
>> + /* 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] 22+ messages in thread* [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
2017-01-25 13:48 ` Shankar, Uma
@ 2017-01-25 14:13 ` Vidya Srinivas
2017-01-31 10:10 ` Srinivas, Vidya
2017-01-31 10:57 ` Mika Kahola
0 siblings, 2 replies; 22+ messages in thread
From: Vidya Srinivas @ 2017-01-25 14:13 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula, ville.syrjala, Vidya Srinivas
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.
v4: Restricting regulator changes to BXT as suggested by
Jani/Mika
v5: Removed redundant read/modify for regulator register as
per Jani's comment. Maintain enable/disable symmetry as per spec.
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
drivers/gpu/drm/i915/intel_dsi.c | 24 ++++++++++++++++++++++++
2 files changed, 31 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..c98234e 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,17 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
intel_disable_dsi_pll(encoder);
intel_enable_dsi_pll(encoder, pipe_config);
+ if (IS_BROXTON(dev_priv)) {
+ /* 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);
+
+ /* Power up DSI regulator */
+ I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT);
+ I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, 0);
+ }
+
intel_dsi_prepare(encoder, pipe_config);
/* Panel Enable over CRC PMIC */
@@ -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,6 +727,17 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder,
intel_dsi_clear_device_ready(encoder);
+ if (IS_BROXTON(dev_priv)) {
+ /* 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);
+
+ /* 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_disable_dsi_pll(encoder);
if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
--
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] 22+ messages in thread* Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
2017-01-25 14:13 ` Vidya Srinivas
@ 2017-01-31 10:10 ` Srinivas, Vidya
2017-01-31 10:57 ` Mika Kahola
1 sibling, 0 replies; 22+ messages in thread
From: Srinivas, Vidya @ 2017-01-31 10:10 UTC (permalink / raw)
To: intel-gfx; +Cc: Nikula, Jani, Syrjala, Ville
Gentle remainder - could you kindly check the patch please? Thank you.
> -----Original Message-----
> From: Srinivas, Vidya
> Sent: Wednesday, January 25, 2017 7:43 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Kahola,
> Mika <mika.kahola@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>
> Subject: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
>
> 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.
>
> v4: Restricting regulator changes to BXT as suggested by Jani/Mika
>
> v5: Removed redundant read/modify for regulator register as per Jani's
> comment. Maintain enable/disable symmetry as per spec.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
> drivers/gpu/drm/i915/intel_dsi.c | 24 ++++++++++++++++++++++++
> 2 files changed, 31 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..c98234e 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,17 @@ static void intel_dsi_pre_enable(struct
> intel_encoder *encoder,
> intel_disable_dsi_pll(encoder);
> intel_enable_dsi_pll(encoder, pipe_config);
>
> + if (IS_BROXTON(dev_priv)) {
> + /* 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);
> +
> + /* Power up DSI regulator */
> + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT);
> + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, 0);
> + }
> +
> intel_dsi_prepare(encoder, pipe_config);
>
> /* Panel Enable over CRC PMIC */
> @@ -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,6 +727,17 @@ static void intel_dsi_post_disable(struct
> intel_encoder *encoder,
>
> intel_dsi_clear_device_ready(encoder);
>
> + if (IS_BROXTON(dev_priv)) {
> + /* 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);
> +
> + /* 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_disable_dsi_pll(encoder);
>
> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> --
> 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
2017-01-25 14:13 ` Vidya Srinivas
2017-01-31 10:10 ` Srinivas, Vidya
@ 2017-01-31 10:57 ` Mika Kahola
2017-02-01 14:49 ` Jani Nikula
1 sibling, 1 reply; 22+ messages in thread
From: Mika Kahola @ 2017-01-31 10:57 UTC (permalink / raw)
To: Vidya Srinivas, intel-gfx; +Cc: jani.nikula, ville.syrjala
Looks ok.
Acked-by: Mika Kahola <mika.kahola@intel.com>
On Wed, 2017-01-25 at 19:43 +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.
>
> v4: Restricting regulator changes to BXT as suggested by
> Jani/Mika
>
> v5: Removed redundant read/modify for regulator register as
> per Jani's comment. Maintain enable/disable symmetry as per spec.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
> drivers/gpu/drm/i915/intel_dsi.c | 24 ++++++++++++++++++++++++
> 2 files changed, 31 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..c98234e 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,17 @@ static void intel_dsi_pre_enable(struct
> intel_encoder *encoder,
> intel_disable_dsi_pll(encoder);
> intel_enable_dsi_pll(encoder, pipe_config);
>
> + if (IS_BROXTON(dev_priv)) {
> + /* 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);
> +
> + /* Power up DSI regulator */
> + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT);
> + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, 0);
> + }
> +
> intel_dsi_prepare(encoder, pipe_config);
>
> /* Panel Enable over CRC PMIC */
> @@ -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,6 +727,17 @@ static void intel_dsi_post_disable(struct
> intel_encoder *encoder,
>
> intel_dsi_clear_device_ready(encoder);
>
> + if (IS_BROXTON(dev_priv)) {
> + /* 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);
> +
> + /* 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_disable_dsi_pll(encoder);
>
> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
--
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] 22+ messages in thread* Re: [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators
2017-01-31 10:57 ` Mika Kahola
@ 2017-02-01 14:49 ` Jani Nikula
0 siblings, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2017-02-01 14:49 UTC (permalink / raw)
To: mika.kahola, Vidya Srinivas, intel-gfx; +Cc: ville.syrjala
On Tue, 31 Jan 2017, Mika Kahola <mika.kahola@intel.com> wrote:
> Looks ok.
>
> Acked-by: Mika Kahola <mika.kahola@intel.com>
Pushed to drm-intel-next-queued, thanks for the patch.
BR,
Jani.
>
> On Wed, 2017-01-25 at 19:43 +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.
>>
>> v4: Restricting regulator changes to BXT as suggested by
>> Jani/Mika
>>
>> v5: Removed redundant read/modify for regulator register as
>> per Jani's comment. Maintain enable/disable symmetry as per spec.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
>> drivers/gpu/drm/i915/intel_dsi.c | 24 ++++++++++++++++++++++++
>> 2 files changed, 31 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..c98234e 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,17 @@ static void intel_dsi_pre_enable(struct
>> intel_encoder *encoder,
>> intel_disable_dsi_pll(encoder);
>> intel_enable_dsi_pll(encoder, pipe_config);
>>
>> + if (IS_BROXTON(dev_priv)) {
>> + /* 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);
>> +
>> + /* Power up DSI regulator */
>> + I915_WRITE(BXT_P_DSI_REGULATOR_CFG, STAP_SELECT);
>> + I915_WRITE(BXT_P_DSI_REGULATOR_TX_CTRL, 0);
>> + }
>> +
>> intel_dsi_prepare(encoder, pipe_config);
>>
>> /* Panel Enable over CRC PMIC */
>> @@ -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,6 +727,17 @@ static void intel_dsi_post_disable(struct
>> intel_encoder *encoder,
>>
>> intel_dsi_clear_device_ready(encoder);
>>
>> + if (IS_BROXTON(dev_priv)) {
>> + /* 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);
>> +
>> + /* 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_disable_dsi_pll(encoder);
>>
>> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
--
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] 22+ messages in thread
end of thread, other threads:[~2017-02-01 14:49 UTC | newest]
Thread overview: 22+ 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
2017-01-19 10:45 [PATCH] drm/i915: Add MIPI_IO WA and program DSI regulators Vidya Srinivas
2017-01-19 11:04 ` Jani Nikula
2017-01-25 13:48 ` Shankar, Uma
2017-01-25 14:13 ` Vidya Srinivas
2017-01-31 10:10 ` Srinivas, Vidya
2017-01-31 10:57 ` Mika Kahola
2017-02-01 14:49 ` Jani Nikula
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.