* [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
@ 2014-09-23 18:06 clinton.a.taylor
2014-09-24 8:19 ` Jani Nikula
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: clinton.a.taylor @ 2014-09-23 18:06 UTC (permalink / raw)
To: Intel-gfx
From: Clint Taylor <clinton.a.taylor@intel.com>
Haswell and later silicon has added a new pixel replication register
to the pipe timings for each transcoder. Now in addition to the
DPLL_A_MD register for the pixel clock double, we also need to write to
the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
to the DPLL only double the pixel clock.
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_display.c | 6 +++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 15c0eaa..7c078d9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2431,6 +2431,7 @@ enum punit_power_well {
#define _PIPEASRC 0x6001c
#define _BCLRPAT_A 0x60020
#define _VSYNCSHIFT_A 0x60028
+#define _MULTIPLY_A 0x6002c
/* Pipe B timing regs */
#define _HTOTAL_B 0x61000
@@ -2442,6 +2443,7 @@ enum punit_power_well {
#define _PIPEBSRC 0x6101c
#define _BCLRPAT_B 0x61020
#define _VSYNCSHIFT_B 0x61028
+#define _MULTIPLY_B 0x6102c
#define TRANSCODER_A_OFFSET 0x60000
#define TRANSCODER_B_OFFSET 0x61000
@@ -2462,6 +2464,7 @@ enum punit_power_well {
#define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
#define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
#define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
+#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
/* HSW+ eDP PSR registers */
#define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c092ff4..e58fcde 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_set_pipe_timings(intel_crtc);
+ I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
+ intel_crtc->config.pixel_multiplier - 1);
+
if (intel_crtc->config.has_pch_encoder) {
intel_cpu_transcoder_set_m_n(intel_crtc,
&intel_crtc->config.fdi_m_n, NULL);
@@ -7811,7 +7814,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
(I915_READ(IPS_CTL) & IPS_ENABLE);
- pipe_config->pixel_multiplier = 1;
+ pipe_config->pixel_multiplier =
+ I915_READ(MULTIPLY(pipe_config->cpu_transcoder)) + 1;
return true;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-23 18:06 [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW clinton.a.taylor
@ 2014-09-24 8:19 ` Jani Nikula
2014-09-24 8:47 ` Daniel Vetter
2014-09-24 8:51 ` Daniel Vetter
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2014-09-24 8:19 UTC (permalink / raw)
To: clinton.a.taylor, Intel-gfx
On Tue, 23 Sep 2014, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> Haswell and later silicon has added a new pixel replication register
> to the pipe timings for each transcoder. Now in addition to the
> DPLL_A_MD register for the pixel clock double, we also need to write to
> the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> to the DPLL only double the pixel clock.
Any idea of the failure modes that this will fix? I'm wondering if we
already have bugs for this.
BR,
Jani.
>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_display.c | 6 +++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 15c0eaa..7c078d9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2431,6 +2431,7 @@ enum punit_power_well {
> #define _PIPEASRC 0x6001c
> #define _BCLRPAT_A 0x60020
> #define _VSYNCSHIFT_A 0x60028
> +#define _MULTIPLY_A 0x6002c
>
> /* Pipe B timing regs */
> #define _HTOTAL_B 0x61000
> @@ -2442,6 +2443,7 @@ enum punit_power_well {
> #define _PIPEBSRC 0x6101c
> #define _BCLRPAT_B 0x61020
> #define _VSYNCSHIFT_B 0x61028
> +#define _MULTIPLY_B 0x6102c
>
> #define TRANSCODER_A_OFFSET 0x60000
> #define TRANSCODER_B_OFFSET 0x61000
> @@ -2462,6 +2464,7 @@ enum punit_power_well {
> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> +#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
>
> /* HSW+ eDP PSR registers */
> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c092ff4..e58fcde 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_set_pipe_timings(intel_crtc);
>
> + I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
> + intel_crtc->config.pixel_multiplier - 1);
> +
> if (intel_crtc->config.has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> &intel_crtc->config.fdi_m_n, NULL);
> @@ -7811,7 +7814,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> (I915_READ(IPS_CTL) & IPS_ENABLE);
>
> - pipe_config->pixel_multiplier = 1;
> + pipe_config->pixel_multiplier =
> + I915_READ(MULTIPLY(pipe_config->cpu_transcoder)) + 1;
>
> return true;
> }
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-24 8:19 ` Jani Nikula
@ 2014-09-24 8:47 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2014-09-24 8:47 UTC (permalink / raw)
To: Jani Nikula; +Cc: Intel-gfx
On Wed, Sep 24, 2014 at 11:19:17AM +0300, Jani Nikula wrote:
> On Tue, 23 Sep 2014, clinton.a.taylor@intel.com wrote:
> > From: Clint Taylor <clinton.a.taylor@intel.com>
> >
> > Haswell and later silicon has added a new pixel replication register
> > to the pipe timings for each transcoder. Now in addition to the
> > DPLL_A_MD register for the pixel clock double, we also need to write to
> > the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> > to the DPLL only double the pixel clock.
>
> Any idea of the failure modes that this will fix? I'm wondering if we
> already have bugs for this.
Should only be possible to hit since we've enabled doubleclocked hdmi
modes with
commit 697c4078c765c02b9c4ca2d828ae4d7af62453a6
Author: Clint Taylor <clinton.a.taylor@intel.com>
Date: Tue Sep 2 17:03:36 2014 -0700
drm/i915/hdmi: Enable pipe pixel replication for SD interlaced modes
Would be impressive if we'd have a hsw/bdw bug report about this already
;-)
Cheers, Daniel
>
> BR,
> Jani.
>
> >
> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > drivers/gpu/drm/i915/intel_display.c | 6 +++++-
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 15c0eaa..7c078d9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2431,6 +2431,7 @@ enum punit_power_well {
> > #define _PIPEASRC 0x6001c
> > #define _BCLRPAT_A 0x60020
> > #define _VSYNCSHIFT_A 0x60028
> > +#define _MULTIPLY_A 0x6002c
> >
> > /* Pipe B timing regs */
> > #define _HTOTAL_B 0x61000
> > @@ -2442,6 +2443,7 @@ enum punit_power_well {
> > #define _PIPEBSRC 0x6101c
> > #define _BCLRPAT_B 0x61020
> > #define _VSYNCSHIFT_B 0x61028
> > +#define _MULTIPLY_B 0x6102c
> >
> > #define TRANSCODER_A_OFFSET 0x60000
> > #define TRANSCODER_B_OFFSET 0x61000
> > @@ -2462,6 +2464,7 @@ enum punit_power_well {
> > #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> > #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> > #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> > +#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> >
> > /* HSW+ eDP PSR registers */
> > #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c092ff4..e58fcde 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >
> > intel_set_pipe_timings(intel_crtc);
> >
> > + I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
> > + intel_crtc->config.pixel_multiplier - 1);
> > +
> > if (intel_crtc->config.has_pch_encoder) {
> > intel_cpu_transcoder_set_m_n(intel_crtc,
> > &intel_crtc->config.fdi_m_n, NULL);
> > @@ -7811,7 +7814,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> > pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> > (I915_READ(IPS_CTL) & IPS_ENABLE);
> >
> > - pipe_config->pixel_multiplier = 1;
> > + pipe_config->pixel_multiplier =
> > + I915_READ(MULTIPLY(pipe_config->cpu_transcoder)) + 1;
> >
> > return true;
> > }
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-23 18:06 [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW clinton.a.taylor
2014-09-24 8:19 ` Jani Nikula
@ 2014-09-24 8:51 ` Daniel Vetter
2014-09-24 15:44 ` Clint Taylor
2014-09-24 23:51 ` [PATCH v2] " clinton.a.taylor
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2014-09-24 8:51 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: Intel-gfx
On Tue, Sep 23, 2014 at 11:06:56AM -0700, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> Haswell and later silicon has added a new pixel replication register
> to the pipe timings for each transcoder. Now in addition to the
> DPLL_A_MD register for the pixel clock double, we also need to write to
> the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> to the DPLL only double the pixel clock.
>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_display.c | 6 +++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 15c0eaa..7c078d9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2431,6 +2431,7 @@ enum punit_power_well {
> #define _PIPEASRC 0x6001c
> #define _BCLRPAT_A 0x60020
> #define _VSYNCSHIFT_A 0x60028
> +#define _MULTIPLY_A 0x6002c
>
> /* Pipe B timing regs */
> #define _HTOTAL_B 0x61000
> @@ -2442,6 +2443,7 @@ enum punit_power_well {
> #define _PIPEBSRC 0x6101c
> #define _BCLRPAT_B 0x61020
> #define _VSYNCSHIFT_B 0x61028
> +#define _MULTIPLY_B 0x6102c
>
> #define TRANSCODER_A_OFFSET 0x60000
> #define TRANSCODER_B_OFFSET 0x61000
> @@ -2462,6 +2464,7 @@ enum punit_power_well {
> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> +#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
MULTIPLY is a bit generic and doesn't even match Bspec lingo. I'd just go
with PIPE_MULTI instead to match Bspec and give it a nice PIPE_ prefix.
>
> /* HSW+ eDP PSR registers */
> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c092ff4..e58fcde 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_set_pipe_timings(intel_crtc);
>
> + I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
This register is per-pipe, so needs to be indexed with intel_crtc->pipe.
Same below.
Otherwise this loooks good.
-Daniel
> + intel_crtc->config.pixel_multiplier - 1);
> +
> if (intel_crtc->config.has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> &intel_crtc->config.fdi_m_n, NULL);
> @@ -7811,7 +7814,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> (I915_READ(IPS_CTL) & IPS_ENABLE);
>
> - pipe_config->pixel_multiplier = 1;
> + pipe_config->pixel_multiplier =
> + I915_READ(MULTIPLY(pipe_config->cpu_transcoder)) + 1;
>
> return true;
> }
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-24 8:51 ` Daniel Vetter
@ 2014-09-24 15:44 ` Clint Taylor
2014-09-24 19:42 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Clint Taylor @ 2014-09-24 15:44 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel-gfx
On 09/24/2014 01:51 AM, Daniel Vetter wrote:
> On Tue, Sep 23, 2014 at 11:06:56AM -0700, clinton.a.taylor@intel.com wrote:
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> Haswell and later silicon has added a new pixel replication register
>> to the pipe timings for each transcoder. Now in addition to the
>> DPLL_A_MD register for the pixel clock double, we also need to write to
>> the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
>> to the DPLL only double the pixel clock.
>>
>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 3 +++
>> drivers/gpu/drm/i915/intel_display.c | 6 +++++-
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 15c0eaa..7c078d9 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2431,6 +2431,7 @@ enum punit_power_well {
>> #define _PIPEASRC 0x6001c
>> #define _BCLRPAT_A 0x60020
>> #define _VSYNCSHIFT_A 0x60028
>> +#define _MULTIPLY_A 0x6002c
>>
>> /* Pipe B timing regs */
>> #define _HTOTAL_B 0x61000
>> @@ -2442,6 +2443,7 @@ enum punit_power_well {
>> #define _PIPEBSRC 0x6101c
>> #define _BCLRPAT_B 0x61020
>> #define _VSYNCSHIFT_B 0x61028
>> +#define _MULTIPLY_B 0x6102c
>>
>> #define TRANSCODER_A_OFFSET 0x60000
>> #define TRANSCODER_B_OFFSET 0x61000
>> @@ -2462,6 +2464,7 @@ enum punit_power_well {
>> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
>> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
>> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
>> +#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
>
> MULTIPLY is a bit generic and doesn't even match Bspec lingo. I'd just go
> with PIPE_MULTI instead to match Bspec and give it a nice PIPE_ prefix.
>>
>> /* HSW+ eDP PSR registers */
>> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index c092ff4..e58fcde 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>>
>> intel_set_pipe_timings(intel_crtc);
>>
>> + I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
>
> This register is per-pipe, so needs to be indexed with intel_crtc->pipe.
> Same below.
>
The MULTIPLY Macro calls the _TRANSCODER2 MACRO which already indexes
the register based on intel_crtc->pipe. This should be all that's required.
-Clint
> Otherwise this loooks good.
> -Daniel
>
>
>> + intel_crtc->config.pixel_multiplier - 1);
>> +
>> if (intel_crtc->config.has_pch_encoder) {
>> intel_cpu_transcoder_set_m_n(intel_crtc,
>> &intel_crtc->config.fdi_m_n, NULL);
>> @@ -7811,7 +7814,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
>> (I915_READ(IPS_CTL) & IPS_ENABLE);
>>
>> - pipe_config->pixel_multiplier = 1;
>> + pipe_config->pixel_multiplier =
>> + I915_READ(MULTIPLY(pipe_config->cpu_transcoder)) + 1;
>>
>> return true;
>> }
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-24 15:44 ` Clint Taylor
@ 2014-09-24 19:42 ` Daniel Vetter
2014-09-25 4:11 ` Ville Syrjälä
0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2014-09-24 19:42 UTC (permalink / raw)
To: Clint Taylor; +Cc: Intel-gfx
On Wed, Sep 24, 2014 at 08:44:42AM -0700, Clint Taylor wrote:
> On 09/24/2014 01:51 AM, Daniel Vetter wrote:
> >On Tue, Sep 23, 2014 at 11:06:56AM -0700, clinton.a.taylor@intel.com wrote:
> >>From: Clint Taylor <clinton.a.taylor@intel.com>
> >>
> >>Haswell and later silicon has added a new pixel replication register
> >>to the pipe timings for each transcoder. Now in addition to the
> >>DPLL_A_MD register for the pixel clock double, we also need to write to
> >>the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> >>to the DPLL only double the pixel clock.
> >>
> >>Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >>---
> >> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> >> drivers/gpu/drm/i915/intel_display.c | 6 +++++-
> >> 2 files changed, 8 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>index 15c0eaa..7c078d9 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -2431,6 +2431,7 @@ enum punit_power_well {
> >> #define _PIPEASRC 0x6001c
> >> #define _BCLRPAT_A 0x60020
> >> #define _VSYNCSHIFT_A 0x60028
> >>+#define _MULTIPLY_A 0x6002c
> >>
> >> /* Pipe B timing regs */
> >> #define _HTOTAL_B 0x61000
> >>@@ -2442,6 +2443,7 @@ enum punit_power_well {
> >> #define _PIPEBSRC 0x6101c
> >> #define _BCLRPAT_B 0x61020
> >> #define _VSYNCSHIFT_B 0x61028
> >>+#define _MULTIPLY_B 0x6102c
> >>
> >> #define TRANSCODER_A_OFFSET 0x60000
> >> #define TRANSCODER_B_OFFSET 0x61000
> >>@@ -2462,6 +2464,7 @@ enum punit_power_well {
> >> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> >> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> >> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> >>+#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> >
> >MULTIPLY is a bit generic and doesn't even match Bspec lingo. I'd just go
> >with PIPE_MULTI instead to match Bspec and give it a nice PIPE_ prefix.
> >>
> >> /* HSW+ eDP PSR registers */
> >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index c092ff4..e58fcde 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >>
> >> intel_set_pipe_timings(intel_crtc);
> >>
> >>+ I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
> >
> >This register is per-pipe, so needs to be indexed with intel_crtc->pipe.
> >Same below.
> >
> The MULTIPLY Macro calls the _TRANSCODER2 MACRO which already indexes the
> register based on intel_crtc->pipe. This should be all that's required.
I don't see where it indexes with intel_crtc->pipe ...
But it doesn't matter since the register is clearly in the transcoder
block, and the reason why Bspec says is per-pipe is that the edp
transcoder doesn't have it. So on second consideration I guess we can keep
this part as-is then.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-23 18:06 [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW clinton.a.taylor
2014-09-24 8:19 ` Jani Nikula
2014-09-24 8:51 ` Daniel Vetter
@ 2014-09-24 23:51 ` clinton.a.taylor
2014-09-25 17:03 ` [PATCH v3] " clinton.a.taylor
2014-09-30 17:30 ` [PATCH v4] " clinton.a.taylor
4 siblings, 0 replies; 18+ messages in thread
From: clinton.a.taylor @ 2014-09-24 23:51 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter, Jani Nikula
From: Clint Taylor <clinton.a.taylor@intel.com>
Haswell and later silicon has added a new pixel replication register
to the pipe timings for each transcoder. Now in addition to the
DPLL_A_MD register for the pixel clock double, we also need to write
to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
to the DPLL only double the pixel clock.
ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_display.c | 6 +++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ad8179b..035d58c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2443,6 +2443,7 @@ enum punit_power_well {
#define _PIPEASRC 0x6001c
#define _BCLRPAT_A 0x60020
#define _VSYNCSHIFT_A 0x60028
+#define _MULTIPLY_A 0x6002c
/* Pipe B timing regs */
#define _HTOTAL_B 0x61000
@@ -2454,6 +2455,7 @@ enum punit_power_well {
#define _PIPEBSRC 0x6101c
#define _BCLRPAT_B 0x61020
#define _VSYNCSHIFT_B 0x61028
+#define _MULTIPLY_B 0x6102c
#define TRANSCODER_A_OFFSET 0x60000
#define TRANSCODER_B_OFFSET 0x61000
@@ -2474,6 +2476,7 @@ enum punit_power_well {
#define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
#define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
#define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
+#define PIPE_MULTI(trans) _TRANSCODER2(trans, _MULTIPLY_A)
/* HSW+ eDP PSR registers */
#define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 858011d..49eced4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4168,6 +4168,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_set_pipe_timings(intel_crtc);
+ I915_WRITE(PIPE_MULTI(intel_crtc->config.cpu_transcoder),
+ intel_crtc->config.pixel_multiplier - 1);
+
if (intel_crtc->config.has_pch_encoder) {
intel_cpu_transcoder_set_m_n(intel_crtc,
&intel_crtc->config.fdi_m_n, NULL);
@@ -7853,7 +7856,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
(I915_READ(IPS_CTL) & IPS_ENABLE);
- pipe_config->pixel_multiplier = 1;
+ pipe_config->pixel_multiplier =
+ I915_READ(PIPE_MULTI(pipe_config->cpu_transcoder)) + 1;
return true;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-24 19:42 ` Daniel Vetter
@ 2014-09-25 4:11 ` Ville Syrjälä
2014-09-29 12:36 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2014-09-25 4:11 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel-gfx
On Wed, Sep 24, 2014 at 09:42:18PM +0200, Daniel Vetter wrote:
> On Wed, Sep 24, 2014 at 08:44:42AM -0700, Clint Taylor wrote:
> > On 09/24/2014 01:51 AM, Daniel Vetter wrote:
> > >On Tue, Sep 23, 2014 at 11:06:56AM -0700, clinton.a.taylor@intel.com wrote:
> > >>From: Clint Taylor <clinton.a.taylor@intel.com>
> > >>
> > >>Haswell and later silicon has added a new pixel replication register
> > >>to the pipe timings for each transcoder. Now in addition to the
> > >>DPLL_A_MD register for the pixel clock double, we also need to write to
> > >>the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> > >>to the DPLL only double the pixel clock.
> > >>
> > >>Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > >>---
> > >> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > >> drivers/gpu/drm/i915/intel_display.c | 6 +++++-
> > >> 2 files changed, 8 insertions(+), 1 deletion(-)
> > >>
> > >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > >>index 15c0eaa..7c078d9 100644
> > >>--- a/drivers/gpu/drm/i915/i915_reg.h
> > >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> > >>@@ -2431,6 +2431,7 @@ enum punit_power_well {
> > >> #define _PIPEASRC 0x6001c
> > >> #define _BCLRPAT_A 0x60020
> > >> #define _VSYNCSHIFT_A 0x60028
> > >>+#define _MULTIPLY_A 0x6002c
> > >>
> > >> /* Pipe B timing regs */
> > >> #define _HTOTAL_B 0x61000
> > >>@@ -2442,6 +2443,7 @@ enum punit_power_well {
> > >> #define _PIPEBSRC 0x6101c
> > >> #define _BCLRPAT_B 0x61020
> > >> #define _VSYNCSHIFT_B 0x61028
> > >>+#define _MULTIPLY_B 0x6102c
> > >>
> > >> #define TRANSCODER_A_OFFSET 0x60000
> > >> #define TRANSCODER_B_OFFSET 0x61000
> > >>@@ -2462,6 +2464,7 @@ enum punit_power_well {
> > >> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> > >> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> > >> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> > >>+#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> > >
> > >MULTIPLY is a bit generic and doesn't even match Bspec lingo. I'd just go
> > >with PIPE_MULTI instead to match Bspec and give it a nice PIPE_ prefix.
> > >>
> > >> /* HSW+ eDP PSR registers */
> > >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> > >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >>index c092ff4..e58fcde 100644
> > >>--- a/drivers/gpu/drm/i915/intel_display.c
> > >>+++ b/drivers/gpu/drm/i915/intel_display.c
> > >>@@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> > >>
> > >> intel_set_pipe_timings(intel_crtc);
> > >>
> > >>+ I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
> > >
> > >This register is per-pipe, so needs to be indexed with intel_crtc->pipe.
> > >Same below.
> > >
> > The MULTIPLY Macro calls the _TRANSCODER2 MACRO which already indexes the
> > register based on intel_crtc->pipe. This should be all that's required.
>
> I don't see where it indexes with intel_crtc->pipe ...
>
> But it doesn't matter since the register is clearly in the transcoder
> block, and the reason why Bspec says is per-pipe is that the edp
> transcoder doesn't have it. So on second consideration I guess we can keep
> this part as-is then.
? If it doesn't exist for EDP we can't just go passing cpu_transcoder
to it.
BDW BSpec seems to claim that it really is a transcoder register and not
a pipe register (just looking at the offset isn't enoguh to tell that
as PIPESRC shows). So in that sense using cpu_transcoder is more
appropriate, but if we do that we must not write the register when
cpu_transcoder == EDP. I suppose that even makes sense since it's only
valid for HDMI/DVI and that's not supported on the EDP transcoder.
But someone really must verify that it really is a transcoder and not a
pipe register and that it has no effect on transcoder EDP.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-23 18:06 [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW clinton.a.taylor
` (2 preceding siblings ...)
2014-09-24 23:51 ` [PATCH v2] " clinton.a.taylor
@ 2014-09-25 17:03 ` clinton.a.taylor
2014-09-26 16:38 ` Ville Syrjälä
2014-09-30 12:12 ` Ville Syrjälä
2014-09-30 17:30 ` [PATCH v4] " clinton.a.taylor
4 siblings, 2 replies; 18+ messages in thread
From: clinton.a.taylor @ 2014-09-25 17:03 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter, Jani Nikula
From: Clint Taylor <clinton.a.taylor@intel.com>
Haswell and later silicon has added a new pixel replication register
to the pipe timings for each transcoder. Now in addition to the
DPLL_A_MD register for the pixel clock double, we also need to write
to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
to the DPLL only double the pixel clock.
ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ad8179b..035d58c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2443,6 +2443,7 @@ enum punit_power_well {
#define _PIPEASRC 0x6001c
#define _BCLRPAT_A 0x60020
#define _VSYNCSHIFT_A 0x60028
+#define _MULTIPLY_A 0x6002c
/* Pipe B timing regs */
#define _HTOTAL_B 0x61000
@@ -2454,6 +2455,7 @@ enum punit_power_well {
#define _PIPEBSRC 0x6101c
#define _BCLRPAT_B 0x61020
#define _VSYNCSHIFT_B 0x61028
+#define _MULTIPLY_B 0x6102c
#define TRANSCODER_A_OFFSET 0x60000
#define TRANSCODER_B_OFFSET 0x61000
@@ -2474,6 +2476,7 @@ enum punit_power_well {
#define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
#define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
#define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
+#define PIPE_MULTI(trans) _TRANSCODER2(trans, _MULTIPLY_A)
/* HSW+ eDP PSR registers */
#define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 858011d..f8c1f11 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_set_pipe_timings(intel_crtc);
+ if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
+ I915_WRITE(PIPE_MULTI(intel_crtc->config.cpu_transcoder),
+ intel_crtc->config.pixel_multiplier - 1);
+ }
+
if (intel_crtc->config.has_pch_encoder) {
intel_cpu_transcoder_set_m_n(intel_crtc,
&intel_crtc->config.fdi_m_n, NULL);
@@ -7853,7 +7858,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
(I915_READ(IPS_CTL) & IPS_ENABLE);
- pipe_config->pixel_multiplier = 1;
+ if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
+ pipe_config->pixel_multiplier =
+ I915_READ(PIPE_MULTI(pipe_config->cpu_transcoder)) + 1;
+ }
return true;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-25 17:03 ` [PATCH v3] " clinton.a.taylor
@ 2014-09-26 16:38 ` Ville Syrjälä
2014-09-26 22:04 ` Clint Taylor
2014-09-30 12:12 ` Ville Syrjälä
1 sibling, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2014-09-26 16:38 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: Jani Nikula, Daniel Vetter, Intel-gfx
On Thu, Sep 25, 2014 at 10:03:53AM -0700, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> Haswell and later silicon has added a new pixel replication register
> to the pipe timings for each transcoder. Now in addition to the
> DPLL_A_MD register for the pixel clock double, we also need to write
> to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> to the DPLL only double the pixel clock.
>
> ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
> ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
>
> Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ad8179b..035d58c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2443,6 +2443,7 @@ enum punit_power_well {
> #define _PIPEASRC 0x6001c
> #define _BCLRPAT_A 0x60020
> #define _VSYNCSHIFT_A 0x60028
> +#define _MULTIPLY_A 0x6002c
>
> /* Pipe B timing regs */
> #define _HTOTAL_B 0x61000
> @@ -2454,6 +2455,7 @@ enum punit_power_well {
> #define _PIPEBSRC 0x6101c
> #define _BCLRPAT_B 0x61020
> #define _VSYNCSHIFT_B 0x61028
> +#define _MULTIPLY_B 0x6102c
>
> #define TRANSCODER_A_OFFSET 0x60000
> #define TRANSCODER_B_OFFSET 0x61000
> @@ -2474,6 +2476,7 @@ enum punit_power_well {
> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> +#define PIPE_MULTI(trans) _TRANSCODER2(trans, _MULTIPLY_A)
>
> /* HSW+ eDP PSR registers */
> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 858011d..f8c1f11 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_set_pipe_timings(intel_crtc);
>
> + if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
> + I915_WRITE(PIPE_MULTI(intel_crtc->config.cpu_transcoder),
> + intel_crtc->config.pixel_multiplier - 1);
> + }
So did you verify that the register really is a transcoder register?
Eg. set PIPE_MULT(A) to >1x and use pipe A to drive the EDP transcoder.
> +
> if (intel_crtc->config.has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> &intel_crtc->config.fdi_m_n, NULL);
> @@ -7853,7 +7858,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> (I915_READ(IPS_CTL) & IPS_ENABLE);
>
> - pipe_config->pixel_multiplier = 1;
> + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> + pipe_config->pixel_multiplier =
> + I915_READ(PIPE_MULTI(pipe_config->cpu_transcoder)) + 1;
> + }
>
> return true;
> }
> --
> 1.7.9.5
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-26 16:38 ` Ville Syrjälä
@ 2014-09-26 22:04 ` Clint Taylor
2014-09-29 12:27 ` Ville Syrjälä
0 siblings, 1 reply; 18+ messages in thread
From: Clint Taylor @ 2014-09-26 22:04 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Jani Nikula, Daniel Vetter, Intel-gfx
On 09/26/2014 09:38 AM, Ville Syrjälä wrote:
> On Thu, Sep 25, 2014 at 10:03:53AM -0700, clinton.a.taylor@intel.com wrote:
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> Haswell and later silicon has added a new pixel replication register
>> to the pipe timings for each transcoder. Now in addition to the
>> DPLL_A_MD register for the pixel clock double, we also need to write
>> to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
>> to the DPLL only double the pixel clock.
>>
>> ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
>> ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
>>
>> Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>>
>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 3 +++
>> drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index ad8179b..035d58c 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2443,6 +2443,7 @@ enum punit_power_well {
>> #define _PIPEASRC 0x6001c
>> #define _BCLRPAT_A 0x60020
>> #define _VSYNCSHIFT_A 0x60028
>> +#define _MULTIPLY_A 0x6002c
>>
>> /* Pipe B timing regs */
>> #define _HTOTAL_B 0x61000
>> @@ -2454,6 +2455,7 @@ enum punit_power_well {
>> #define _PIPEBSRC 0x6101c
>> #define _BCLRPAT_B 0x61020
>> #define _VSYNCSHIFT_B 0x61028
>> +#define _MULTIPLY_B 0x6102c
>>
>> #define TRANSCODER_A_OFFSET 0x60000
>> #define TRANSCODER_B_OFFSET 0x61000
>> @@ -2474,6 +2476,7 @@ enum punit_power_well {
>> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
>> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
>> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
>> +#define PIPE_MULTI(trans) _TRANSCODER2(trans, _MULTIPLY_A)
>>
>> /* HSW+ eDP PSR registers */
>> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 858011d..f8c1f11 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>>
>> intel_set_pipe_timings(intel_crtc);
>>
>> + if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
>> + I915_WRITE(PIPE_MULTI(intel_crtc->config.cpu_transcoder),
>> + intel_crtc->config.pixel_multiplier - 1);
>> + }
>
> So did you verify that the register really is a transcoder register?
> Eg. set PIPE_MULT(A) to >1x and use pipe A to drive the EDP transcoder.
I did not verify. This change was done based on the fact that the
register does not exist in the VPG HTML version of the BPEC for
Transcoder_EDP, only TRANS_MULT_A, _B, and _C are defined.
Do we have an SI contact that can confirm?
-Clint
>
>> +
>> if (intel_crtc->config.has_pch_encoder) {
>> intel_cpu_transcoder_set_m_n(intel_crtc,
>> &intel_crtc->config.fdi_m_n, NULL);
>> @@ -7853,7 +7858,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
>> (I915_READ(IPS_CTL) & IPS_ENABLE);
>>
>> - pipe_config->pixel_multiplier = 1;
>> + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
>> + pipe_config->pixel_multiplier =
>> + I915_READ(PIPE_MULTI(pipe_config->cpu_transcoder)) + 1;
>> + }
>>
>> return true;
>> }
>> --
>> 1.7.9.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-26 22:04 ` Clint Taylor
@ 2014-09-29 12:27 ` Ville Syrjälä
2014-09-29 17:02 ` Runyan, Arthur J
0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2014-09-29 12:27 UTC (permalink / raw)
To: Clint Taylor; +Cc: Jani Nikula, Daniel Vetter, Intel-gfx, Runyan, Arthur J
On Fri, Sep 26, 2014 at 03:04:22PM -0700, Clint Taylor wrote:
> On 09/26/2014 09:38 AM, Ville Syrjälä wrote:
> > On Thu, Sep 25, 2014 at 10:03:53AM -0700, clinton.a.taylor@intel.com wrote:
> >> From: Clint Taylor <clinton.a.taylor@intel.com>
> >>
> >> Haswell and later silicon has added a new pixel replication register
> >> to the pipe timings for each transcoder. Now in addition to the
> >> DPLL_A_MD register for the pixel clock double, we also need to write
> >> to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> >> to the DPLL only double the pixel clock.
> >>
> >> ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
> >> ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
> >>
> >> Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >>
> >> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> >> drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
> >> 2 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index ad8179b..035d58c 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -2443,6 +2443,7 @@ enum punit_power_well {
> >> #define _PIPEASRC 0x6001c
> >> #define _BCLRPAT_A 0x60020
> >> #define _VSYNCSHIFT_A 0x60028
> >> +#define _MULTIPLY_A 0x6002c
> >>
> >> /* Pipe B timing regs */
> >> #define _HTOTAL_B 0x61000
> >> @@ -2454,6 +2455,7 @@ enum punit_power_well {
> >> #define _PIPEBSRC 0x6101c
> >> #define _BCLRPAT_B 0x61020
> >> #define _VSYNCSHIFT_B 0x61028
> >> +#define _MULTIPLY_B 0x6102c
> >>
> >> #define TRANSCODER_A_OFFSET 0x60000
> >> #define TRANSCODER_B_OFFSET 0x61000
> >> @@ -2474,6 +2476,7 @@ enum punit_power_well {
> >> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> >> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> >> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> >> +#define PIPE_MULTI(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> >>
> >> /* HSW+ eDP PSR registers */
> >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 858011d..f8c1f11 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >>
> >> intel_set_pipe_timings(intel_crtc);
> >>
> >> + if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
> >> + I915_WRITE(PIPE_MULTI(intel_crtc->config.cpu_transcoder),
> >> + intel_crtc->config.pixel_multiplier - 1);
> >> + }
> >
> > So did you verify that the register really is a transcoder register?
> > Eg. set PIPE_MULT(A) to >1x and use pipe A to drive the EDP transcoder.
>
> I did not verify. This change was done based on the fact that the
> register does not exist in the VPG HTML version of the BPEC for
> Transcoder_EDP, only TRANS_MULT_A, _B, and _C are defined.
>
> Do we have an SI contact that can confirm?
Cc:ing Art.
Art, the confusion here is whether PIPE_MULT is a transcoder register
or a pipe register. BSpec seems to be telling us that it's a transcoder
register but the confusion comes from the fact that the EDP transcoder
doesn't have this register. My theory is that it is a transcoder register,
but since pixel repeat isn't needed for eDP the register isn't present
(or relevant) in the EDP transcoder. Can you clarify this?
Although in this case it would be very easy to test this theory on
actual hardware as I previously suggested.
>
> -Clint
>
>
> >
> >> +
> >> if (intel_crtc->config.has_pch_encoder) {
> >> intel_cpu_transcoder_set_m_n(intel_crtc,
> >> &intel_crtc->config.fdi_m_n, NULL);
> >> @@ -7853,7 +7858,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> >> (I915_READ(IPS_CTL) & IPS_ENABLE);
> >>
> >> - pipe_config->pixel_multiplier = 1;
> >> + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> >> + pipe_config->pixel_multiplier =
> >> + I915_READ(PIPE_MULTI(pipe_config->cpu_transcoder)) + 1;
> >> + }
> >>
> >> return true;
> >> }
> >> --
> >> 1.7.9.5
> >
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-25 4:11 ` Ville Syrjälä
@ 2014-09-29 12:36 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2014-09-29 12:36 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On Thu, Sep 25, 2014 at 07:11:11AM +0300, Ville Syrjälä wrote:
> On Wed, Sep 24, 2014 at 09:42:18PM +0200, Daniel Vetter wrote:
> > On Wed, Sep 24, 2014 at 08:44:42AM -0700, Clint Taylor wrote:
> > > On 09/24/2014 01:51 AM, Daniel Vetter wrote:
> > > >On Tue, Sep 23, 2014 at 11:06:56AM -0700, clinton.a.taylor@intel.com wrote:
> > > >>From: Clint Taylor <clinton.a.taylor@intel.com>
> > > >>
> > > >>Haswell and later silicon has added a new pixel replication register
> > > >>to the pipe timings for each transcoder. Now in addition to the
> > > >>DPLL_A_MD register for the pixel clock double, we also need to write to
> > > >>the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> > > >>to the DPLL only double the pixel clock.
> > > >>
> > > >>Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > > >>---
> > > >> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > > >> drivers/gpu/drm/i915/intel_display.c | 6 +++++-
> > > >> 2 files changed, 8 insertions(+), 1 deletion(-)
> > > >>
> > > >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > >>index 15c0eaa..7c078d9 100644
> > > >>--- a/drivers/gpu/drm/i915/i915_reg.h
> > > >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> > > >>@@ -2431,6 +2431,7 @@ enum punit_power_well {
> > > >> #define _PIPEASRC 0x6001c
> > > >> #define _BCLRPAT_A 0x60020
> > > >> #define _VSYNCSHIFT_A 0x60028
> > > >>+#define _MULTIPLY_A 0x6002c
> > > >>
> > > >> /* Pipe B timing regs */
> > > >> #define _HTOTAL_B 0x61000
> > > >>@@ -2442,6 +2443,7 @@ enum punit_power_well {
> > > >> #define _PIPEBSRC 0x6101c
> > > >> #define _BCLRPAT_B 0x61020
> > > >> #define _VSYNCSHIFT_B 0x61028
> > > >>+#define _MULTIPLY_B 0x6102c
> > > >>
> > > >> #define TRANSCODER_A_OFFSET 0x60000
> > > >> #define TRANSCODER_B_OFFSET 0x61000
> > > >>@@ -2462,6 +2464,7 @@ enum punit_power_well {
> > > >> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> > > >> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> > > >> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> > > >>+#define MULTIPLY(trans) _TRANSCODER2(trans, _MULTIPLY_A)
> > > >
> > > >MULTIPLY is a bit generic and doesn't even match Bspec lingo. I'd just go
> > > >with PIPE_MULTI instead to match Bspec and give it a nice PIPE_ prefix.
> > > >>
> > > >> /* HSW+ eDP PSR registers */
> > > >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> > > >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > >>index c092ff4..e58fcde 100644
> > > >>--- a/drivers/gpu/drm/i915/intel_display.c
> > > >>+++ b/drivers/gpu/drm/i915/intel_display.c
> > > >>@@ -4152,6 +4152,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> > > >>
> > > >> intel_set_pipe_timings(intel_crtc);
> > > >>
> > > >>+ I915_WRITE(MULTIPLY(intel_crtc->config.cpu_transcoder),
> > > >
> > > >This register is per-pipe, so needs to be indexed with intel_crtc->pipe.
> > > >Same below.
> > > >
> > > The MULTIPLY Macro calls the _TRANSCODER2 MACRO which already indexes the
> > > register based on intel_crtc->pipe. This should be all that's required.
> >
> > I don't see where it indexes with intel_crtc->pipe ...
> >
> > But it doesn't matter since the register is clearly in the transcoder
> > block, and the reason why Bspec says is per-pipe is that the edp
> > transcoder doesn't have it. So on second consideration I guess we can keep
> > this part as-is then.
>
> ? If it doesn't exist for EDP we can't just go passing cpu_transcoder
> to it.
>
> BDW BSpec seems to claim that it really is a transcoder register and not
> a pipe register (just looking at the offset isn't enoguh to tell that
> as PIPESRC shows). So in that sense using cpu_transcoder is more
> appropriate, but if we do that we must not write the register when
> cpu_transcoder == EDP. I suppose that even makes sense since it's only
> valid for HDMI/DVI and that's not supported on the EDP transcoder.
> But someone really must verify that it really is a transcoder and not a
> pipe register and that it has no effect on transcoder EDP.
I guess we could use the cpu_transcoder and add a WARN_ON(cpu_transcoder
== EDP). Makes stuff consistent and if we ever botch this up we'll know.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-29 12:27 ` Ville Syrjälä
@ 2014-09-29 17:02 ` Runyan, Arthur J
0 siblings, 0 replies; 18+ messages in thread
From: Runyan, Arthur J @ 2014-09-29 17:02 UTC (permalink / raw)
To: Ville Syrjälä, Taylor, Clinton A
Cc: Nikula, Jani, Daniel Vetter, Intel-gfx
>> > So did you verify that the register really is a transcoder register?
>> > Eg. set PIPE_MULT(A) to >1x and use pipe A to drive the EDP transcoder.
>>
>> I did not verify. This change was done based on the fact that the
>> register does not exist in the VPG HTML version of the BPEC for
>> Transcoder_EDP, only TRANS_MULT_A, _B, and _C are defined.
>>
>> Do we have an SI contact that can confirm?
>
>Cc:ing Art.
>
>Art, the confusion here is whether PIPE_MULT is a transcoder register
>or a pipe register. BSpec seems to be telling us that it's a transcoder
>register but the confusion comes from the fact that the EDP transcoder
>doesn't have this register. My theory is that it is a transcoder register,
>but since pixel repeat isn't needed for eDP the register isn't present
>(or relevant) in the EDP transcoder. Can you clarify this?
>
>Although in this case it would be very easy to test this theory on
>actual hardware as I previously suggested.
You are correct. It's transcoder based. It only gets used in HDMI/DVI modes, so EDP doesn't get one. Broadwell was able to properly rename transcoder stuff, so these became TRANS_MULT.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-25 17:03 ` [PATCH v3] " clinton.a.taylor
2014-09-26 16:38 ` Ville Syrjälä
@ 2014-09-30 12:12 ` Ville Syrjälä
1 sibling, 0 replies; 18+ messages in thread
From: Ville Syrjälä @ 2014-09-30 12:12 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: Jani Nikula, Daniel Vetter, Intel-gfx
On Thu, Sep 25, 2014 at 10:03:53AM -0700, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> Haswell and later silicon has added a new pixel replication register
> to the pipe timings for each transcoder. Now in addition to the
> DPLL_A_MD register for the pixel clock double, we also need to write
> to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> to the DPLL only double the pixel clock.
>
> ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
> ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
>
> Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ad8179b..035d58c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2443,6 +2443,7 @@ enum punit_power_well {
> #define _PIPEASRC 0x6001c
> #define _BCLRPAT_A 0x60020
> #define _VSYNCSHIFT_A 0x60028
> +#define _MULTIPLY_A 0x6002c
>
> /* Pipe B timing regs */
> #define _HTOTAL_B 0x61000
> @@ -2454,6 +2455,7 @@ enum punit_power_well {
> #define _PIPEBSRC 0x6101c
> #define _BCLRPAT_B 0x61020
> #define _VSYNCSHIFT_B 0x61028
> +#define _MULTIPLY_B 0x6102c
>
> #define TRANSCODER_A_OFFSET 0x60000
> #define TRANSCODER_B_OFFSET 0x61000
> @@ -2474,6 +2476,7 @@ enum punit_power_well {
> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> +#define PIPE_MULTI(trans) _TRANSCODER2(trans, _MULTIPLY_A)
PIPE_MULT (w/o the 'I') is what the spec called it. Best to follow the
same naming conventiom to make it easier to search for things in the spec.
I would also name the _MULTIPLY_x defines the same way.
>
> /* HSW+ eDP PSR registers */
> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 858011d..f8c1f11 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_set_pipe_timings(intel_crtc);
>
> + if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
> + I915_WRITE(PIPE_MULTI(intel_crtc->config.cpu_transcoder),
> + intel_crtc->config.pixel_multiplier - 1);
> + }
> +
> if (intel_crtc->config.has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> &intel_crtc->config.fdi_m_n, NULL);
> @@ -7853,7 +7858,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> (I915_READ(IPS_CTL) & IPS_ENABLE);
>
> - pipe_config->pixel_multiplier = 1;
> + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> + pipe_config->pixel_multiplier =
> + I915_READ(PIPE_MULTI(pipe_config->cpu_transcoder)) + 1;
> + }
else
pixel_multiplier = 1;
With those fixed/changed this look good.
>
> return true;
> }
> --
> 1.7.9.5
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-23 18:06 [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW clinton.a.taylor
` (3 preceding siblings ...)
2014-09-25 17:03 ` [PATCH v3] " clinton.a.taylor
@ 2014-09-30 17:30 ` clinton.a.taylor
2014-09-30 18:22 ` Ville Syrjälä
4 siblings, 1 reply; 18+ messages in thread
From: clinton.a.taylor @ 2014-09-30 17:30 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter, Jani Nikula
From: Clint Taylor <clinton.a.taylor@intel.com>
Haswell and later silicon has added a new pixel replication register
to the pipe timings for each transcoder. Now in addition to the
DPLL_A_MD register for the pixel clock double, we also need to write
to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
to the DPLL only double the pixel clock.
ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
ver4: Macro name change to PIPE_MULT and default else pixel_multiplier
Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ad8179b..d428f7a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2443,6 +2443,7 @@ enum punit_power_well {
#define _PIPEASRC 0x6001c
#define _BCLRPAT_A 0x60020
#define _VSYNCSHIFT_A 0x60028
+#define _PIPE_MULT_A 0x6002c
/* Pipe B timing regs */
#define _HTOTAL_B 0x61000
@@ -2454,6 +2455,7 @@ enum punit_power_well {
#define _PIPEBSRC 0x6101c
#define _BCLRPAT_B 0x61020
#define _VSYNCSHIFT_B 0x61028
+#define _PIPE_MULT_B 0x6102c
#define TRANSCODER_A_OFFSET 0x60000
#define TRANSCODER_B_OFFSET 0x61000
@@ -2474,6 +2476,7 @@ enum punit_power_well {
#define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
#define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
#define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
+#define PIPE_MULT(trans) _TRANSCODER2(trans, _PIPE_MULT_A)
/* HSW+ eDP PSR registers */
#define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 858011d..617dad6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_set_pipe_timings(intel_crtc);
+ if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
+ I915_WRITE(PIPE_MULT(intel_crtc->config.cpu_transcoder),
+ intel_crtc->config.pixel_multiplier - 1);
+ }
+
if (intel_crtc->config.has_pch_encoder) {
intel_cpu_transcoder_set_m_n(intel_crtc,
&intel_crtc->config.fdi_m_n, NULL);
@@ -7853,7 +7858,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
(I915_READ(IPS_CTL) & IPS_ENABLE);
- pipe_config->pixel_multiplier = 1;
+ if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
+ pipe_config->pixel_multiplier =
+ I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1;
+ }
+ else {
+ pipe_config->pixel_multiplier = 1;
+ }
return true;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v4] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-30 17:30 ` [PATCH v4] " clinton.a.taylor
@ 2014-09-30 18:22 ` Ville Syrjälä
2014-10-01 8:03 ` Daniel Vetter
0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2014-09-30 18:22 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: Jani Nikula, Daniel Vetter, Intel-gfx
On Tue, Sep 30, 2014 at 10:30:22AM -0700, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> Haswell and later silicon has added a new pixel replication register
> to the pipe timings for each transcoder. Now in addition to the
> DPLL_A_MD register for the pixel clock double, we also need to write
> to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> to the DPLL only double the pixel clock.
>
> ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
> ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
> ver4: Macro name change to PIPE_MULT and default else pixel_multiplier
>
> Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ad8179b..d428f7a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2443,6 +2443,7 @@ enum punit_power_well {
> #define _PIPEASRC 0x6001c
> #define _BCLRPAT_A 0x60020
> #define _VSYNCSHIFT_A 0x60028
> +#define _PIPE_MULT_A 0x6002c
>
> /* Pipe B timing regs */
> #define _HTOTAL_B 0x61000
> @@ -2454,6 +2455,7 @@ enum punit_power_well {
> #define _PIPEBSRC 0x6101c
> #define _BCLRPAT_B 0x61020
> #define _VSYNCSHIFT_B 0x61028
> +#define _PIPE_MULT_B 0x6102c
>
> #define TRANSCODER_A_OFFSET 0x60000
> #define TRANSCODER_B_OFFSET 0x61000
> @@ -2474,6 +2476,7 @@ enum punit_power_well {
> #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> +#define PIPE_MULT(trans) _TRANSCODER2(trans, _PIPE_MULT_A)
>
> /* HSW+ eDP PSR registers */
> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 858011d..617dad6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_set_pipe_timings(intel_crtc);
>
> + if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
> + I915_WRITE(PIPE_MULT(intel_crtc->config.cpu_transcoder),
> + intel_crtc->config.pixel_multiplier - 1);
> + }
> +
> if (intel_crtc->config.has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> &intel_crtc->config.fdi_m_n, NULL);
> @@ -7853,7 +7858,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> (I915_READ(IPS_CTL) & IPS_ENABLE);
>
> - pipe_config->pixel_multiplier = 1;
> + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> + pipe_config->pixel_multiplier =
> + I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1;
> + }
> + else {
> + pipe_config->pixel_multiplier = 1;
> + }
A bit too many curly braces for my taste but maybe that's just me.
In any case looks like it will DTRT so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> return true;
> }
> --
> 1.7.9.5
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4] drm/i915: Enable pixel replicated modes on BDW and HSW.
2014-09-30 18:22 ` Ville Syrjälä
@ 2014-10-01 8:03 ` Daniel Vetter
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2014-10-01 8:03 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Daniel Vetter, Intel-gfx, Jani Nikula
On Tue, Sep 30, 2014 at 09:22:25PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 30, 2014 at 10:30:22AM -0700, clinton.a.taylor@intel.com wrote:
> > From: Clint Taylor <clinton.a.taylor@intel.com>
> >
> > Haswell and later silicon has added a new pixel replication register
> > to the pipe timings for each transcoder. Now in addition to the
> > DPLL_A_MD register for the pixel clock double, we also need to write
> > to the TRANS_MULT_n (0x6002c) register to double the pixel data. Writing
> > to the DPLL only double the pixel clock.
> >
> > ver2: Macro name change from MULTIPLY to PIPE_MULTI. (Daniel)
> > ver3: Do not set pixel multiplier if transcoder is eDP (Ville)
> > ver4: Macro name change to PIPE_MULT and default else pixel_multiplier
> >
> > Cc: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> >
> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++++-
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index ad8179b..d428f7a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2443,6 +2443,7 @@ enum punit_power_well {
> > #define _PIPEASRC 0x6001c
> > #define _BCLRPAT_A 0x60020
> > #define _VSYNCSHIFT_A 0x60028
> > +#define _PIPE_MULT_A 0x6002c
> >
> > /* Pipe B timing regs */
> > #define _HTOTAL_B 0x61000
> > @@ -2454,6 +2455,7 @@ enum punit_power_well {
> > #define _PIPEBSRC 0x6101c
> > #define _BCLRPAT_B 0x61020
> > #define _VSYNCSHIFT_B 0x61028
> > +#define _PIPE_MULT_B 0x6102c
> >
> > #define TRANSCODER_A_OFFSET 0x60000
> > #define TRANSCODER_B_OFFSET 0x61000
> > @@ -2474,6 +2476,7 @@ enum punit_power_well {
> > #define BCLRPAT(trans) _TRANSCODER2(trans, _BCLRPAT_A)
> > #define VSYNCSHIFT(trans) _TRANSCODER2(trans, _VSYNCSHIFT_A)
> > #define PIPESRC(trans) _TRANSCODER2(trans, _PIPEASRC)
> > +#define PIPE_MULT(trans) _TRANSCODER2(trans, _PIPE_MULT_A)
> >
> > /* HSW+ eDP PSR registers */
> > #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 858011d..617dad6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4168,6 +4168,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >
> > intel_set_pipe_timings(intel_crtc);
> >
> > + if (intel_crtc->config.cpu_transcoder != TRANSCODER_EDP) {
> > + I915_WRITE(PIPE_MULT(intel_crtc->config.cpu_transcoder),
> > + intel_crtc->config.pixel_multiplier - 1);
> > + }
> > +
> > if (intel_crtc->config.has_pch_encoder) {
> > intel_cpu_transcoder_set_m_n(intel_crtc,
> > &intel_crtc->config.fdi_m_n, NULL);
> > @@ -7853,7 +7858,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> > pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
> > (I915_READ(IPS_CTL) & IPS_ENABLE);
> >
> > - pipe_config->pixel_multiplier = 1;
> > + if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> > + pipe_config->pixel_multiplier =
> > + I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1;
> > + }
> > + else {
> > + pipe_config->pixel_multiplier = 1;
> > + }
>
> A bit too many curly braces for my taste but maybe that's just me.
> In any case looks like it will DTRT so:
Checkpatch actually complained about the misaligned in the first hunk and
about the } on a line of its own. And with proper kernel coding style
braces are allowed on single-statement multi-line blocks and should be
applied to all blocks of an if statement if one has them. So it's totally
ok.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Queued for -next-fixes, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-10-01 8:04 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-23 18:06 [PATCH] drm/i915: Enable pixel replicated modes on BDW and HSW clinton.a.taylor
2014-09-24 8:19 ` Jani Nikula
2014-09-24 8:47 ` Daniel Vetter
2014-09-24 8:51 ` Daniel Vetter
2014-09-24 15:44 ` Clint Taylor
2014-09-24 19:42 ` Daniel Vetter
2014-09-25 4:11 ` Ville Syrjälä
2014-09-29 12:36 ` Daniel Vetter
2014-09-24 23:51 ` [PATCH v2] " clinton.a.taylor
2014-09-25 17:03 ` [PATCH v3] " clinton.a.taylor
2014-09-26 16:38 ` Ville Syrjälä
2014-09-26 22:04 ` Clint Taylor
2014-09-29 12:27 ` Ville Syrjälä
2014-09-29 17:02 ` Runyan, Arthur J
2014-09-30 12:12 ` Ville Syrjälä
2014-09-30 17:30 ` [PATCH v4] " clinton.a.taylor
2014-09-30 18:22 ` Ville Syrjälä
2014-10-01 8:03 ` Daniel Vetter
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.