Re: [PATCH 4/7] drm/i915: Use the default 600ns LDO programming sequence delay

From: Deepak S <deepak.s@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/7] drm/i915: Use the default 600ns LDO programming sequence delay
Date: Fri, 08 May 2015 19:05:13 +0530	[thread overview]
Message-ID: <554CBB91.7050709@linux.intel.com> (raw)
In-Reply-To: <20150508132248.GF18908@intel.com>


On Friday 08 May 2015 06:52 PM, Ville Syrjälä wrote:
> On Fri, May 08, 2015 at 06:31:23PM +0530, Deepak S wrote:
>>
>> On Friday 10 April 2015 08:51 PM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Not sure which LDO programming sequence delay should be used for the CHV
>>> PHY, but the spec says that 600ns is "Used by default for initial
>>> bringup", and the BIOS seems to use that, so let's do the same.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_reg.h         | 4 ++++
>>>    drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
>>>    2 files changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 98588d5..977bad6 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -1887,6 +1887,10 @@ enum skl_disp_power_wells {
>>>    #define DPIO_PHY_STATUS			(VLV_DISPLAY_BASE + 0x6240)
>>>    #define   DPLL_PORTD_READY_MASK		(0xf)
>>>    #define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
>>> +#define   PHY_LDO_DELAY_0NS			0x0
>>> +#define   PHY_LDO_DELAY_200NS			0x1
>> PHY_LDO_DELAY_0NS & PHY_LDO_DELAY_200NS not used right?
>> Should we keep the definitions?
> I generally like to keep a bit of extra for VLV/CHV due to the bad doc
> situation.
>
>>> +#define   PHY_LDO_DELAY_600NS			0x2
>>> +#define   PHY_LDO_SEQ_DELAY(delay, phy)		((delay) << (2*(phy)+23))
>>>    #define   PHY_CH_SU_PSR				0x1
>>>    #define   PHY_CH_DEEP_PSR			0x7
>>>    #define   PHY_CH_POWER_MODE(mode, phy, ch)	((mode) << (6*(phy)+3*(ch)+2))
>>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> index 1f800f8..5cd8a51 100644
>>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> @@ -1406,6 +1406,8 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>>>    	 * value.
>>>    	 */
>>>    	dev_priv->chv_phy_control =
>>> +		PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY0) |
>>> +		PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY1) |
>>>    		PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH0) |
>>>    		PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH1) |
>>>    		PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY1, DPIO_CH0);
>> I think we need to squash this patch to previous one?
>> [Intel-gfx] [PATCH 1/7] drm/i915: Implement chv display PHY lane stagger setup
>> http://www.spinics.net/lists/intel-gfx/msg64481.html
> Well, IIRC I never saw any real issues with the 0ns delay either, with
> or without the lane stagger setup. So not much point in squashing IMO.
>

Thanks for the clarification :)
Reviewed-by:  Deepak S<deepak.s@linux.intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx