diff mbox

[25/33] drm/i915: PLL and clock gating registers need an offset on VLV

Message ID 1359034198-19678-26-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Jan. 24, 2013, 1:29 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Daniel Vetter Jan. 24, 2013, 10:41 p.m. UTC | #1 
On Thu, Jan 24, 2013 at 03:29:50PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1c1e7f8..c3d4ddc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -922,8 +922,8 @@
>  #define   VGA1_PD_P1_DIV_2	(1 << 13)
>  #define   VGA1_PD_P1_SHIFT	8
>  #define   VGA1_PD_P1_MASK	(0x1f << 8)
> -#define _DPLL_A	0x06014
> -#define _DPLL_B	0x06018
> +#define _DPLL_A	(dev_priv->info->display_mmio_offset + 0x6014)
> +#define _DPLL_B	(dev_priv->info->display_mmio_offset + 0x6018)
>  #define DPLL(pipe) _PIPE(pipe, _DPLL_A, _DPLL_B)
>  #define   DPLL_VCO_ENABLE		(1 << 31)
>  #define   DPLL_DVO_HIGH_SPEED		(1 << 30)
> @@ -982,7 +982,7 @@
>  #define   SDVO_MULTIPLIER_MASK			0x000000ff
>  #define   SDVO_MULTIPLIER_SHIFT_HIRES		4
>  #define   SDVO_MULTIPLIER_SHIFT_VGA		0
> -#define _DPLL_A_MD 0x0601c /* 965+ only */
> +#define _DPLL_A_MD (dev_priv->info->display_mmio_offset + 0x601c) /* 965+ only */
>  /*
>   * UDI pixel divider, controlling how many pixels are stuffed into a packet.
>   *
> @@ -1019,7 +1019,7 @@
>   */
>  #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
>  #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
> -#define _DPLL_B_MD 0x06020 /* 965+ only */
> +#define _DPLL_B_MD (dev_priv->info->display_mmio_offset + 0x6020) /* 965+ only */
>  #define DPLL_MD(pipe) _PIPE(pipe, _DPLL_A_MD, _DPLL_B_MD)
>  
>  #define _FPA0	0x06040
> @@ -1047,12 +1047,12 @@
>  #define   DPLLA_TEST_N_BYPASS		(1 << 3)
>  #define   DPLLA_TEST_M_BYPASS		(1 << 2)
>  #define   DPLLA_INPUT_BUFFER_ENABLE	(1 << 0)
> -#define D_STATE		0x6104
> +#define D_STATE		(dev_priv->info->display_mmio_offset + 0x6104)

I only see this one here used in gen2/3 code ...

>  #define  DSTATE_GFX_RESET_I830			(1<<6)
>  #define  DSTATE_PLL_D3_OFF			(1<<3)
>  #define  DSTATE_GFX_CLOCK_GATING		(1<<1)
>  #define  DSTATE_DOT_CLOCK_GATING		(1<<0)
> -#define DSPCLK_GATE_D		0x6200
> +#define DSPCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6200)

This one here seems to be only used up to gen4 ...

>  # define DPUNIT_B_CLOCK_GATE_DISABLE		(1 << 30) /* 965 */
>  # define VSUNIT_CLOCK_GATE_DISABLE		(1 << 29) /* 965 */
>  # define VRHUNIT_CLOCK_GATE_DISABLE		(1 << 28) /* 965 */
> @@ -1159,7 +1159,7 @@
>  #define VF_UNIT_CLOCK_GATE_DISABLE		(1 << 9)
>  #define GS_UNIT_CLOCK_GATE_DISABLE		(1 << 7)
>  #define CL_UNIT_CLOCK_GATE_DISABLE		(1 << 6)
> -#define RAMCLK_GATE_D		0x6210		/* CRL only */
> +#define RAMCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6210)		/* CRL only */

And same here. Do we need those in vlv code, or can we just drop them for
now?
-Daniel

>  #define DEUC			0x6214          /* CRL only */
>  
>  #define FW_BLC_SELF_VLV		(VLV_DISPLAY_BASE + 0x6500)
> -- 
> 1.7.12.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Jan. 25, 2013, 10:51 a.m. UTC | #2 
On Thu, Jan 24, 2013 at 11:41:53PM +0100, Daniel Vetter wrote:
> On Thu, Jan 24, 2013 at 03:29:50PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 1c1e7f8..c3d4ddc 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -922,8 +922,8 @@
> >  #define   VGA1_PD_P1_DIV_2	(1 << 13)
> >  #define   VGA1_PD_P1_SHIFT	8
> >  #define   VGA1_PD_P1_MASK	(0x1f << 8)
> > -#define _DPLL_A	0x06014
> > -#define _DPLL_B	0x06018
> > +#define _DPLL_A	(dev_priv->info->display_mmio_offset + 0x6014)
> > +#define _DPLL_B	(dev_priv->info->display_mmio_offset + 0x6018)
> >  #define DPLL(pipe) _PIPE(pipe, _DPLL_A, _DPLL_B)
> >  #define   DPLL_VCO_ENABLE		(1 << 31)
> >  #define   DPLL_DVO_HIGH_SPEED		(1 << 30)
> > @@ -982,7 +982,7 @@
> >  #define   SDVO_MULTIPLIER_MASK			0x000000ff
> >  #define   SDVO_MULTIPLIER_SHIFT_HIRES		4
> >  #define   SDVO_MULTIPLIER_SHIFT_VGA		0
> > -#define _DPLL_A_MD 0x0601c /* 965+ only */
> > +#define _DPLL_A_MD (dev_priv->info->display_mmio_offset + 0x601c) /* 965+ only */
> >  /*
> >   * UDI pixel divider, controlling how many pixels are stuffed into a packet.
> >   *
> > @@ -1019,7 +1019,7 @@
> >   */
> >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
> >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
> > -#define _DPLL_B_MD 0x06020 /* 965+ only */
> > +#define _DPLL_B_MD (dev_priv->info->display_mmio_offset + 0x6020) /* 965+ only */
> >  #define DPLL_MD(pipe) _PIPE(pipe, _DPLL_A_MD, _DPLL_B_MD)
> >  
> >  #define _FPA0	0x06040
> > @@ -1047,12 +1047,12 @@
> >  #define   DPLLA_TEST_N_BYPASS		(1 << 3)
> >  #define   DPLLA_TEST_M_BYPASS		(1 << 2)
> >  #define   DPLLA_INPUT_BUFFER_ENABLE	(1 << 0)
> > -#define D_STATE		0x6104
> > +#define D_STATE		(dev_priv->info->display_mmio_offset + 0x6104)
> 
> I only see this one here used in gen2/3 code ...
> 
> >  #define  DSTATE_GFX_RESET_I830			(1<<6)
> >  #define  DSTATE_PLL_D3_OFF			(1<<3)
> >  #define  DSTATE_GFX_CLOCK_GATING		(1<<1)
> >  #define  DSTATE_DOT_CLOCK_GATING		(1<<0)
> > -#define DSPCLK_GATE_D		0x6200
> > +#define DSPCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6200)
> 
> This one here seems to be only used up to gen4 ...

DSPCLK_GATE_D is used in intel_i2c_quirk_set(). OTOH gma500 has the
same code commented out, so it may be that we can skip it too. Anyone
have more details on this quirk?

gma500 also seems to use DSPCLK_GATE_D to disable clock gating for
some DP stuff on CDV. Considering the lineage we need to find out
if that's something that affects VLV as well.

> >  # define DPUNIT_B_CLOCK_GATE_DISABLE		(1 << 30) /* 965 */
> >  # define VSUNIT_CLOCK_GATE_DISABLE		(1 << 29) /* 965 */
> >  # define VRHUNIT_CLOCK_GATE_DISABLE		(1 << 28) /* 965 */
> > @@ -1159,7 +1159,7 @@
> >  #define VF_UNIT_CLOCK_GATE_DISABLE		(1 << 9)
> >  #define GS_UNIT_CLOCK_GATE_DISABLE		(1 << 7)
> >  #define CL_UNIT_CLOCK_GATE_DISABLE		(1 << 6)
> > -#define RAMCLK_GATE_D		0x6210		/* CRL only */
> > +#define RAMCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6210)		/* CRL only */
> 
> And same here. Do we need those in vlv code, or can we just drop them for
> now?

I think D_STATE and RAMCLK_GATE_D can be dropped. We can convert them
if and when we actaully have to touch them.
Daniel Vetter Jan. 25, 2013, 4:06 p.m. UTC | #3 
On Fri, Jan 25, 2013 at 12:51:15PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 24, 2013 at 11:41:53PM +0100, Daniel Vetter wrote:
> > On Thu, Jan 24, 2013 at 03:29:50PM +0200, ville.syrjala@linux.intel.com wrote:
> > >  #define  DSTATE_GFX_RESET_I830			(1<<6)
> > >  #define  DSTATE_PLL_D3_OFF			(1<<3)
> > >  #define  DSTATE_GFX_CLOCK_GATING		(1<<1)
> > >  #define  DSTATE_DOT_CLOCK_GATING		(1<<0)
> > > -#define DSPCLK_GATE_D		0x6200
> > > +#define DSPCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6200)
> > 
> > This one here seems to be only used up to gen4 ...
> 
> DSPCLK_GATE_D is used in intel_i2c_quirk_set(). OTOH gma500 has the
> same code commented out, so it may be that we can skip it too. Anyone
> have more details on this quirk?

Afaict that quirk is for pnv only.

> gma500 also seems to use DSPCLK_GATE_D to disable clock gating for
> some DP stuff on CDV. Considering the lineage we need to find out
> if that's something that affects VLV as well.

I guess we could add it once we need it in a vlv clock gating functions.
Generally we tend to only add clock gating defines when we need them,
since there are soooooo many.
-Daniel
Ville Syrjälä Jan. 25, 2013, 4:20 p.m. UTC | #4 
On Fri, Jan 25, 2013 at 05:06:51PM +0100, Daniel Vetter wrote:
> On Fri, Jan 25, 2013 at 12:51:15PM +0200, Ville Syrjälä wrote:
> > On Thu, Jan 24, 2013 at 11:41:53PM +0100, Daniel Vetter wrote:
> > > On Thu, Jan 24, 2013 at 03:29:50PM +0200, ville.syrjala@linux.intel.com wrote:
> > > >  #define  DSTATE_GFX_RESET_I830			(1<<6)
> > > >  #define  DSTATE_PLL_D3_OFF			(1<<3)
> > > >  #define  DSTATE_GFX_CLOCK_GATING		(1<<1)
> > > >  #define  DSTATE_DOT_CLOCK_GATING		(1<<0)
> > > > -#define DSPCLK_GATE_D		0x6200
> > > > +#define DSPCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6200)
> > > 
> > > This one here seems to be only used up to gen4 ...
> > 
> > DSPCLK_GATE_D is used in intel_i2c_quirk_set(). OTOH gma500 has the
> > same code commented out, so it may be that we can skip it too. Anyone
> > have more details on this quirk?
> 
> Afaict that quirk is for pnv only.

So, should we just kill it for other HW? At least it needs to be killed
for VLV if we don't merge this hunk.

> > gma500 also seems to use DSPCLK_GATE_D to disable clock gating for
> > some DP stuff on CDV. Considering the lineage we need to find out
> > if that's something that affects VLV as well.
> 
> I guess we could add it once we need it in a vlv clock gating functions.
> Generally we tend to only add clock gating defines when we need them,
> since there are soooooo many.

Yeah. I need to write down my notes somewhere so we don't forget all
these questions that arose from this patch set.
Daniel Vetter Jan. 25, 2013, 4:24 p.m. UTC | #5 
On Fri, Jan 25, 2013 at 5:20 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>> > DSPCLK_GATE_D is used in intel_i2c_quirk_set(). OTOH gma500 has the
>> > same code commented out, so it may be that we can skip it too. Anyone
>> > have more details on this quirk?
>>
>> Afaict that quirk is for pnv only.
>
> So, should we just kill it for other HW? At least it needs to be killed
> for VLV if we don't merge this hunk.

There's a


	/* When using bit bashing for I2C, this bit needs to be set to 1 */
	if (!IS_PINEVIEW(dev_priv->dev))
		return;

guard in the quirk function, hence why it's pnv-only. So nothing to
fix I think, or I'm blind again ;-)
-Daniel
Ville Syrjälä Jan. 25, 2013, 4:28 p.m. UTC | #6 
On Fri, Jan 25, 2013 at 06:20:53PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 25, 2013 at 05:06:51PM +0100, Daniel Vetter wrote:
> > On Fri, Jan 25, 2013 at 12:51:15PM +0200, Ville Syrjälä wrote:
> > > On Thu, Jan 24, 2013 at 11:41:53PM +0100, Daniel Vetter wrote:
> > > > On Thu, Jan 24, 2013 at 03:29:50PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > >  #define  DSTATE_GFX_RESET_I830			(1<<6)
> > > > >  #define  DSTATE_PLL_D3_OFF			(1<<3)
> > > > >  #define  DSTATE_GFX_CLOCK_GATING		(1<<1)
> > > > >  #define  DSTATE_DOT_CLOCK_GATING		(1<<0)
> > > > > -#define DSPCLK_GATE_D		0x6200
> > > > > +#define DSPCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6200)
> > > > 
> > > > This one here seems to be only used up to gen4 ...
> > > 
> > > DSPCLK_GATE_D is used in intel_i2c_quirk_set(). OTOH gma500 has the
> > > same code commented out, so it may be that we can skip it too. Anyone
> > > have more details on this quirk?
> > 
> > Afaict that quirk is for pnv only.
> 
> So, should we just kill it for other HW? At least it needs to be killed
> for VLV if we don't merge this hunk.

Ah, it already is disabled for everything else. Great, less work for
me.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1c1e7f8..c3d4ddc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -922,8 +922,8 @@ 
 #define   VGA1_PD_P1_DIV_2	(1 << 13)
 #define   VGA1_PD_P1_SHIFT	8
 #define   VGA1_PD_P1_MASK	(0x1f << 8)
-#define _DPLL_A	0x06014
-#define _DPLL_B	0x06018
+#define _DPLL_A	(dev_priv->info->display_mmio_offset + 0x6014)
+#define _DPLL_B	(dev_priv->info->display_mmio_offset + 0x6018)
 #define DPLL(pipe) _PIPE(pipe, _DPLL_A, _DPLL_B)
 #define   DPLL_VCO_ENABLE		(1 << 31)
 #define   DPLL_DVO_HIGH_SPEED		(1 << 30)
@@ -982,7 +982,7 @@ 
 #define   SDVO_MULTIPLIER_MASK			0x000000ff
 #define   SDVO_MULTIPLIER_SHIFT_HIRES		4
 #define   SDVO_MULTIPLIER_SHIFT_VGA		0
-#define _DPLL_A_MD 0x0601c /* 965+ only */
+#define _DPLL_A_MD (dev_priv->info->display_mmio_offset + 0x601c) /* 965+ only */
 /*
  * UDI pixel divider, controlling how many pixels are stuffed into a packet.
  *
@@ -1019,7 +1019,7 @@ 
  */
 #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
 #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
-#define _DPLL_B_MD 0x06020 /* 965+ only */
+#define _DPLL_B_MD (dev_priv->info->display_mmio_offset + 0x6020) /* 965+ only */
 #define DPLL_MD(pipe) _PIPE(pipe, _DPLL_A_MD, _DPLL_B_MD)
 
 #define _FPA0	0x06040
@@ -1047,12 +1047,12 @@ 
 #define   DPLLA_TEST_N_BYPASS		(1 << 3)
 #define   DPLLA_TEST_M_BYPASS		(1 << 2)
 #define   DPLLA_INPUT_BUFFER_ENABLE	(1 << 0)
-#define D_STATE		0x6104
+#define D_STATE		(dev_priv->info->display_mmio_offset + 0x6104)
 #define  DSTATE_GFX_RESET_I830			(1<<6)
 #define  DSTATE_PLL_D3_OFF			(1<<3)
 #define  DSTATE_GFX_CLOCK_GATING		(1<<1)
 #define  DSTATE_DOT_CLOCK_GATING		(1<<0)
-#define DSPCLK_GATE_D		0x6200
+#define DSPCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6200)
 # define DPUNIT_B_CLOCK_GATE_DISABLE		(1 << 30) /* 965 */
 # define VSUNIT_CLOCK_GATE_DISABLE		(1 << 29) /* 965 */
 # define VRHUNIT_CLOCK_GATE_DISABLE		(1 << 28) /* 965 */
@@ -1159,7 +1159,7 @@ 
 #define VF_UNIT_CLOCK_GATE_DISABLE		(1 << 9)
 #define GS_UNIT_CLOCK_GATE_DISABLE		(1 << 7)
 #define CL_UNIT_CLOCK_GATE_DISABLE		(1 << 6)
-#define RAMCLK_GATE_D		0x6210		/* CRL only */
+#define RAMCLK_GATE_D		(dev_priv->info->display_mmio_offset + 0x6210)		/* CRL only */
 #define DEUC			0x6214          /* CRL only */
 
 #define FW_BLC_SELF_VLV		(VLV_DISPLAY_BASE + 0x6500)