From: sonika <sonika.jindal@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, rodrigo.vivi@intel.com
Subject: Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
Date: Thu, 29 Jan 2015 09:27:14 +0530 [thread overview]
Message-ID: <54C9AF9A.4060404@intel.com> (raw)
In-Reply-To: <20150128160227.GY4764@phenom.ffwll.local>
On Wednesday 28 January 2015 09:32 PM, Daniel Vetter wrote:
> On Thu, Jan 22, 2015 at 02:30:54PM +0530, Sonika Jindal wrote:
>> Mainly taking care of some register offsets, otherwise things are similar to
>> hsw. Also, programming ddi aux to use hardcoded values for psr data select.
>>
>> v2: introduce EDP_PSR_AUX_BASE macro (Chris)
>> v3: Moving to HW tracking for SKL+ platforms, so activating source psr during
>> psr_enabling and then avoiding psr entries and exits for each frontbuffer
>> updates.
>> v4: Using SKL DDI AUX regs instead of changing PSR_AUX regs definition (Rodrigo)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>> drivers/gpu/drm/i915/i915_reg.h | 5 +++++
>> drivers/gpu/drm/i915/intel_frontbuffer.c | 7 +++++--
>> drivers/gpu/drm/i915/intel_psr.c | 26 ++++++++++++++++++++++++--
>> 4 files changed, 36 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 66f0c60..3d24872 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2371,7 +2371,8 @@ struct drm_i915_cmd_table {
>> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
>> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
>> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
>> - IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
>> + IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
>> + IS_SKYLAKE(dev))
>> #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
>> IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
>> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index a39bb03..a6f06fa 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3748,6 +3748,11 @@ enum punit_power_well {
>> #define DP_AUX_CH_CTL_PRECHARGE_TEST (1 << 11)
>> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK (0x7ff)
>> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT 0
>> +#define DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL (1 << 14)
>> +#define DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL (1 << 13)
>> +#define DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL (1 << 12)
>> +#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
>> +#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>> #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
>>
>> /*
>> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> index 79f6d72..010d550 100644
>> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> @@ -156,7 +156,9 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>>
>> intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
>>
>> - intel_psr_invalidate(dev, obj->frontbuffer_bits);
>> +
>> + if (INTEL_INFO(dev)->gen < 9)
>> + intel_psr_invalidate(dev, obj->frontbuffer_bits);
>> }
>>
>> /**
>> @@ -182,7 +184,8 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>>
>> intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
>>
>> - intel_psr_flush(dev, frontbuffer_bits);
>> + if (INTEL_INFO(dev)->gen < 9)
>> + intel_psr_flush(dev, frontbuffer_bits);
> Again no, not going to take wholesale filtering of the sw invalidate
> paths. This needs to be properly tested and pushed down into the psr
> specific invalidate/flush functions on a per-function basis.
>
> I've dropped these two hunks and merged the patch.
> -Daniel
Hi Daniel,
Even SW tracking doesn't work in many cases, like I reported earlier in
ubuntu login screen where we don't get frontbuffer flushes and we don't
enter PSR at all with SW tracking.
I see similar behavior even in fbcon mode. So, I am not sure how you can
say that SW tracking is the only right way.
If there are cases where HW tracking fails (and I know a few), we need
to fix them. I can move this gen check to the intel_psr_* function if
that is the major concern.
-Sonika
>>
>> /*
>> * FIXME: Unconditional fbc flushing here is a rather gross hack and
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index dd0e6e0..4867d5a 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -142,6 +142,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>> struct drm_device *dev = dig_port->base.base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> uint32_t aux_clock_divider;
>> + uint32_t aux_data_reg, aux_ctl_reg;
>> int precharge = 0x3;
>> bool only_standby = dev_priv->vbt.psr.full_link;
>> static const uint8_t aux_msg[] = {
>> @@ -168,16 +169,34 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>> DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>>
>> + aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
>> + DPA_AUX_CH_DATA1 : EDP_PSR_AUX_DATA1(dev);
>> + aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
>> + DPA_AUX_CH_CTL : EDP_PSR_AUX_CTL(dev);
>> +
>> /* Setup AUX registers */
>> for (i = 0; i < sizeof(aux_msg); i += 4)
>> - I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
>> + I915_WRITE(aux_data_reg + i,
>> intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
>>
>> - I915_WRITE(EDP_PSR_AUX_CTL(dev),
>> + if (INTEL_INFO(dev)->gen >= 9) {
>> + uint32_t val;
>> +
>> + val = I915_READ(aux_ctl_reg);
>> + val &= ~DP_AUX_CH_CTL_TIME_OUT_MASK;
>> + val |= DP_AUX_CH_CTL_TIME_OUT_1600us;
>> + val &= ~DP_AUX_CH_CTL_MESSAGE_SIZE_MASK;
>> + val |= (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
>> + /* Use hardcoded data values for PSR */
>> + val &= ~DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL;
>> + I915_WRITE(aux_ctl_reg, val);
>> + } else {
>> + I915_WRITE(aux_ctl_reg,
>> DP_AUX_CH_CTL_TIME_OUT_400us |
>> (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
>> (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>> (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
>> + }
>> }
>>
>> static void vlv_psr_enable_source(struct intel_dp *intel_dp)
>> @@ -355,6 +374,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>>
>> /* Enable PSR on the panel */
>> hsw_psr_enable_sink(intel_dp);
>> +
>> + if (INTEL_INFO(dev)->gen >= 9)
>> + intel_psr_activate(intel_dp);
>> } else {
>> vlv_psr_setup_vsc(intel_dp);
>>
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-01-29 4:04 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-22 9:00 [PATCH] drm/i915/skl: Enabling PSR on Skylake Sonika Jindal
2015-01-22 17:48 ` shuang.he
2015-01-28 16:02 ` Daniel Vetter
2015-01-29 3:57 ` sonika [this message]
2015-01-29 16:11 ` Daniel Vetter
-- strict thread matches above, loose matches on Subject: below --
2015-01-16 8:37 Sonika Jindal
2015-01-16 15:06 ` shuang.he
2015-01-17 4:24 ` Daniel Vetter
2015-01-19 11:40 ` sonika
2015-01-20 9:50 ` Daniel Vetter
2015-01-20 11:49 ` Jindal, Sonika
2015-01-20 22:01 ` Rodrigo Vivi
2015-01-21 4:53 ` sonika
2015-01-21 8:37 ` Daniel Vetter
2015-01-21 8:55 ` sonika
2015-01-21 9:42 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54C9AF9A.4060404@intel.com \
--to=sonika.jindal@intel.com \
--cc=daniel@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=rodrigo.vivi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox