Sean,

On 07/15/2016 05:32 PM, Yakir Yang wrote:
> Sean,
>
> On 07/14/2016 11:23 PM, Sean Paul wrote:
>> On Thu, Jul 14, 2016 at 12:15:53PM +0800, Yakir Yang wrote:
>>> The full name of PSR is Panel Self Refresh, panel device could refresh
>>> itself with the hardware framebuffer in panel, this would make lots of
>>> sense to save the power consumption.
>>>
>>> This patch have exported two symbols for platform driver to implement
>>> the PSR function in hardware side:
>>> - analogix_dp_active_psr()
>>> - analogix_dp_inactive_psr()
>>>
>>> Signed-off-by: Yakir Yang<ykk@rock-chips.com>
>>> ---
>>> Changes in v4:
>>> - Downgrade the PSR version print message to debug level. (Sean)
>>> - Return 'void' instead of 'int' in analogix_dp_enable_sink_psr(). (Sean)
>>> - Delete the unused read dpcd operations in analogix_dp_enable_sink_psr(). (Sean)
>>> - Delete the arbitrary usleep_range in analogix_dp_enable_psr_crc. (Sean).
>>> - Clean up the hardcoded values in analogix_dp_send_psr_spd(). (Sean)
>>> - Rename "active/inactive" to "enable/disable". (Sean, Dominik)
>>> - Keep set the PSR_VID_CRC_FLUSH gate in analogix_dp_enable_psr_crc().
>>>
>>> Changes in v3:
>>> - split analogix_dp_enable_psr(), make it more clearly
>>>      analogix_dp_detect_sink_psr()
>>>      analogix_dp_enable_sink_psr()
>>> - remove some nosie register setting comments
>>>
>>> Changes in v2:
>>> - introduce in v2, splite the common Analogix DP changes out
>>>
>>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 60 ++++++++++++++++++++++
>>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++
>>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 49 ++++++++++++++++++
>>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  | 28 ++++++++++
>>>   include/drm/bridge/analogix_dp.h                   |  3 ++
>>>   5 files changed, 144 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index 32715da..1fec91a 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -97,6 +97,62 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
>>>   	return 0;
>>>   }
>>>   
>>> +int analogix_dp_enable_psr(struct device *dev)
>>> +{
>>> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
>>> +
>>> +	if (!dp->psr_support)
>>> +		return -EINVAL;
>>> +
>>> +	analogix_dp_send_psr_spd(dp, EDP_VSC_PSR_STATE_ACTIVE |
>>> +				 EDP_VSC_PSR_CRC_VALUES_VALID);
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>>> +
>>> +int analogix_dp_disable_psr(struct device *dev)
>>> +{
>>> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
>>> +
>>> +	if (!dp->psr_support)
>>> +		return -EINVAL;
>>> +
>>> +	analogix_dp_send_psr_spd(dp, 0);
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>>> +
>>> +static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp)
>>> +{
>>> +	unsigned char psr_version;
>>> +
>>> +	analogix_dp_read_byte_from_dpcd(dp, DP_PSR_SUPPORT, &psr_version);
>>> +	dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
>>> +
>>> +	return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
>>> +}
>>> +
>>> +static void analogix_dp_enable_sink_psr(struct analogix_dp_device *dp)
>>> +{
>>> +	unsigned char psr_en;
>>> +
>>> +	/* Disable psr function */
>>> +	analogix_dp_read_byte_from_dpcd(dp, DP_PSR_EN_CFG, &psr_en);
>>> +	psr_en &= ~DP_PSR_ENABLE;
>>> +	analogix_dp_write_byte_to_dpcd(dp, DP_PSR_EN_CFG, psr_en);
>>> +
>>> +	/* Main-Link transmitter remains active during PSR active states */
>>> +	psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION;
>>> +	analogix_dp_write_byte_to_dpcd(dp, DP_PSR_EN_CFG, psr_en);
>>> +
>>> +	/* Enable psr function */
>>> +	psr_en = DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE |
>>> +		 DP_PSR_CRC_VERIFICATION;
>>> +	analogix_dp_write_byte_to_dpcd(dp, DP_PSR_EN_CFG, psr_en);
>>> +
>>> +	analogix_dp_enable_psr_crc(dp);
>>> +}
>>> +
>>>   static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
>>>   {
>>>   	int i;
>>> @@ -921,6 +977,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
>>>   
>>>   	/* Enable video */
>>>   	analogix_dp_start_video(dp);
>>> +
>>> +	dp->psr_support = analogix_dp_detect_sink_psr(dp);
>>> +	if (dp->psr_support)
>>> +		analogix_dp_enable_sink_psr(dp);
>>>   }
>>>   
>>>   int analogix_dp_get_modes(struct drm_connector *connector)
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> index b456380..6ca5dde 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> @@ -177,6 +177,7 @@ struct analogix_dp_device {
>>>   	int			hpd_gpio;
>>>   	bool                    force_hpd;
>>>   	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
>>> +	bool			psr_support;
>>>   
>>>   	struct analogix_dp_plat_data *plat_data;
>>>   };
>>> @@ -278,4 +279,7 @@ int analogix_dp_is_video_stream_on(struct analogix_dp_device *dp);
>>>   void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>>>   void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>>>   void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>>> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>>> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, int db1);
>>> +
>>>   #endif /* _ANALOGIX_DP_CORE_H */
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>>> index 48030f0..26446ae 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>>> @@ -26,6 +26,9 @@
>>>   #define COMMON_INT_MASK_4	(HOTPLUG_CHG | HPD_LOST | PLUG)
>>>   #define INT_STA_MASK		INT_HPD
>>>   
>>> +static u32 psr_spd_hbx[] = { 0x00, 0x07, 0x02, 0x08 };
>>> +static u32 psr_spd_pbx[] = { 0x00, 0x16, 0xCE, 0x5D };
>> These need to be const. Can you also please add a comment explaining where you
>> got the values from?
>
> A, after reading the eDP 1.4 spec, HBx values are come from spec, and 
> I also found there is no need to configure the PBx registers (cause 
> spec haven't mentioned it, and my panel still works without it).

Something wrong, I can remove the PBx setting, otherwise panel wouldn't 
enter into PSR active mode.

BR,
- Yakir

>
> *
> HB2: Revision Number*
> Support for 04h and 05h values added in eDP v1.4a.
> 01h = VSC SDP supports only 3D stereo.
> 02h = 3D stereo + PSR.
> 03h = 3D stereo + PSR2.
> 04h = 3D stereo + PSR/PSR2 + Y-coordinate.
> 05h = -3D stereo- + PSR/PSR2 + Y-coordinate + Pixel 
> Encoding/Colorimetry Format indication.
>
> *HB3: Number of Valid Data Bytes*
> 01h = VSC SDP supports only 3D stereo (HB2 = 01h).
> 08h = VSC SDP supporting 3D stereo + PSR (HB2 = 02h).
> 0Ch = VSC SDP supporting 3D stereo + PSR2 (HB2 = 03h), without 
> Y-coordinate of first scan
> line in the SU region.
> 0Eh = VSC SDP supporting 3D stereo + PSR2 (HB2 = 04h), with 
> Y-coordinate of first scan line
> in the SU region.
> 13h = VSC SDP supporting 3D stereo, + PSR2, + Pixel 
> Encoding/Colorimetry Format indication (HB2 = 05h).
>
>
>>> +
>>>   void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable)
>>>   {
>>>   	u32 reg;
>>> @@ -1322,3 +1325,49 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp)
>>>   	reg |= SCRAMBLING_DISABLE;
>>>   	writel(reg, dp->reg_base + ANALOGIX_DP_TRAINING_PTN_SET);
>>>   }
>>> +
>>> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>>> +{
>>> +	writel(PSR_VID_CRC_FLUSH | PSR_VID_CRC_ENABLE,
>>> +	       dp->reg_base + ANALOGIX_DP_CRC_CON);
>>> +}
>>> +
>
> And I found I need to remove the  PSR_VID_CRC_FLUSH enable flag, cause 
> it would caused PSR failed.
>
> Thanks,
> - Yakir
>
>>> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, int db1)
>>> +{
>>> +	unsigned int val;
>>> +	unsigned int i;
>>> +
>>> +	/* don't send info frame */
>>> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>> +	val &= ~IF_EN;
>>> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>> +
>>> +	/* configure single frame update mode */
>>> +	writel(PSR_FRAME_UP_TYPE_BURST | PSR_CRC_SEL_HARDWARE,
>>> +	       dp->reg_base + ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL);
>>> +
>>> +	/* configure VSC HB0~HB3 and PB0~PB3 */
>>> +	for (i = 0; i < 4; i++)
>>> +		writel(psr_spd_hbx[i], dp->reg_base + ANALOGIX_DP_SPD_HB + i*4);
>>> +	for (i = 0; i < 4; i++)
>>> +		writel(psr_spd_pbx[i], dp->reg_base + ANALOGIX_DP_SPD_PB + i*4);
>>> +
>>> +	/* configure DB0 / DB1 values */
>>> +	writel(0x0, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB0);
>>> +	writel(db1, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB1);
>>> +
>>> +	/* set reuse spd inforframe */
>>> +	val = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
>>> +	val |= REUSE_SPD_EN;
>>> +	writel(val, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
>>> +
>>> +	/* mark info frame update */
>>> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>> +	val = (val | IF_UP) & ~IF_EN;
>>> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>> +
>>> +	/* send info frame */
>>> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>> +	val |= IF_EN;
>>> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>>> +}
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
>>> index cdcc6c5..fd232b2 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
>>> @@ -22,6 +22,8 @@
>>>   #define ANALOGIX_DP_VIDEO_CTL_8			0x3C
>>>   #define ANALOGIX_DP_VIDEO_CTL_10		0x44
>>>   
>>> +#define ANALOGIX_DP_SPDIF_AUDIO_CTL_0		0xD8
>>> +
>>>   #define ANALOGIX_DP_PLL_REG_1			0xfc
>>>   #define ANALOGIX_DP_PLL_REG_2			0x9e4
>>>   #define ANALOGIX_DP_PLL_REG_3			0x9e8
>>> @@ -30,6 +32,15 @@
>>>   
>>>   #define ANALOGIX_DP_PD				0x12c
>>>   
>>> +#define ANALOGIX_DP_IF_TYPE			0x244
>>> +#define ANALOGIX_DP_IF_PKT_DB1			0x254
>>> +#define ANALOGIX_DP_IF_PKT_DB2			0x258
>>> +#define ANALOGIX_DP_SPD_HB			0x2F8
>>> +#define ANALOGIX_DP_SPD_PB			0x308
>>> +#define ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL	0x318
>>> +#define ANALOGIX_DP_VSC_SHADOW_DB0		0x31C
>>> +#define ANALOGIX_DP_VSC_SHADOW_DB1		0x320
>>> +
>>>   #define ANALOGIX_DP_LANE_MAP			0x35C
>>>   
>>>   #define ANALOGIX_DP_ANALOG_CTL_1		0x370
>>> @@ -103,6 +114,8 @@
>>>   
>>>   #define ANALOGIX_DP_SOC_GENERAL_CTL		0x800
>>>   
>>> +#define ANALOGIX_DP_CRC_CON			0x890
>>> +
>>>   /* ANALOGIX_DP_TX_SW_RESET */
>>>   #define RESET_DP_TX				(0x1 << 0)
>>>   
>>> @@ -151,6 +164,7 @@
>>>   #define VID_CHK_UPDATE_TYPE_SHIFT		(4)
>>>   #define VID_CHK_UPDATE_TYPE_1			(0x1 << 4)
>>>   #define VID_CHK_UPDATE_TYPE_0			(0x0 << 4)
>>> +#define REUSE_SPD_EN				(0x1 << 3)
>>>   
>>>   /* ANALOGIX_DP_VIDEO_CTL_8 */
>>>   #define VID_HRES_TH(x)				(((x) & 0xf) << 4)
>>> @@ -167,6 +181,12 @@
>>>   #define REF_CLK_27M				(0x0 << 0)
>>>   #define REF_CLK_MASK				(0x1 << 0)
>>>   
>>> +/* ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL */
>>> +#define PSR_FRAME_UP_TYPE_BURST			(0x1 << 0)
>>> +#define PSR_FRAME_UP_TYPE_SINGLE		(0x0 << 0)
>>> +#define PSR_CRC_SEL_HARDWARE			(0x1 << 1)
>>> +#define PSR_CRC_SEL_MANUALLY			(0x0 << 1)
>>> +
>>>   /* ANALOGIX_DP_LANE_MAP */
>>>   #define LANE3_MAP_LOGIC_LANE_0			(0x0 << 6)
>>>   #define LANE3_MAP_LOGIC_LANE_1			(0x1 << 6)
>>> @@ -376,4 +396,12 @@
>>>   #define VIDEO_MODE_SLAVE_MODE			(0x1 << 0)
>>>   #define VIDEO_MODE_MASTER_MODE			(0x0 << 0)
>>>   
>>> +/* ANALOGIX_DP_PKT_SEND_CTL */
>>> +#define IF_UP					(0x1 << 4)
>>> +#define IF_EN					(0x1 << 0)
>>> +
>>> +/* ANALOGIX_DP_CRC_CON */
>>> +#define PSR_VID_CRC_FLUSH			(0x1 << 2)
>>> +#define PSR_VID_CRC_ENABLE			(0x1 << 0)
>>> +
>>>   #endif /* _ANALOGIX_DP_REG_H */
>>> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
>>> index 261b86d..9cd8838 100644
>>> --- a/include/drm/bridge/analogix_dp.h
>>> +++ b/include/drm/bridge/analogix_dp.h
>>> @@ -38,6 +38,9 @@ struct analogix_dp_plat_data {
>>>   			 struct drm_connector *);
>>>   };
>>>   
>>> +int analogix_dp_enable_psr(struct device *dev);
>>> +int analogix_dp_disable_psr(struct device *dev);
>>> +
>>>   int analogix_dp_resume(struct device *dev);
>>>   int analogix_dp_suspend(struct device *dev);
>>>   
>>> -- 
>>> 1.9.1
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>