lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6aaa866c-696c-167c-dbd0-c8da15337f83@rock-chips.com>
Date:	Wed, 17 Aug 2016 14:32:53 +0800
From:	Yakir Yang <ykk@...k-chips.com>
To:	Archit Taneja <architt@...eaurora.org>,
	Mark Yao <yzq@...k-chips.com>, Inki Dae <inki.dae@...sung.com>,
	Thierry Reding <treding@...dia.com>,
	Heiko Stuebner <heiko@...ech.de>
Cc:	Jingoo Han <jingoohan1@...il.com>,
	Javier Martinez Canillas <javier@....samsung.com>,
	Stéphane Marchesin <marcheu@...omium.org>,
	Sean Paul <seanpaul@...omium.org>,
	Tomasz Figa <tfiga@...omium.org>, dianders@...omium.org,
	David Airlie <airlied@...ux.ie>, daniel.vetter@...ll.ch,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	emil.l.velikov@...il.com,
	Gustavo Padovan <gustavo.padovan@...labora.co.uk>,
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	linux-samsung-soc@...r.kernel.org,
	linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v5 3/4] drm/bridge: analogix_dp: add the PSR function
 support

Hello Archit,

On 08/17/2016 01:41 PM, Archit Taneja wrote:
> Hi,
>
> On 07/24/2016 12:27 PM, 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()
>
> Could this in any way mess things up if the dev_type is EXYNOS_DP?
>

Nop, I have enabled the panel PSR function by default (if driver detect 
panel support PSR), but this would active the PSR (cause eDP controller 
haven't send the right SDP header), which means panel would stay in 
normal refresh mode on Exynos platform.

I also have tested that case on RK3399 platform. Enable the panel PSR, 
but never call the analogix_dp_active_psr() symbols, and them I found 
panel still refresh normally.

Only when platform driver call the 
analogix_dp_active_psr()/analogix_dp_inactive_psr() symbols in vblank 
time, make the controller to send the active SDP header, then panel 
could enter into PSR mode.

> Otherwise,
>
> Reviewed-by: Archit Taneja <architt@...eaurora.org>

Thanks for your reviewed.

- Yakir

>
>>
>> Signed-off-by: Yakir Yang <ykk@...k-chips.com>
>> Reviewed-by: Sean Paul <seanpaul@...omium.org>
>> ---
>> Changes in v5:
>> - Add reviewed flag from Sean.
>>
>> Changes in v4.1:
>> - Take use of existing edp_psr_vsc struct to swap HBx and DBx 
>> setting. (Sean)
>> - Remove PSR_VID_CRC_FLUSH setting analogix_dp_enable_psr_crc().
>> - Add comment about PBx magic numbers. (Sean)
>>
>> 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 | 81 
>> ++++++++++++++++++++++
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 ++
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 51 ++++++++++++++
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  | 34 +++++++++
>>   include/drm/bridge/analogix_dp.h                   |  3 +
>>   5 files changed, 174 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..381b25e 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -97,6 +97,83 @@ 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);
>> +    struct edp_vsc_psr psr_vsc;
>> +
>> +    if (!dp->psr_support)
>> +        return -EINVAL;
>> +
>> +    /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
>> +    memset(&psr_vsc, 0, sizeof(psr_vsc));
>> +    psr_vsc.sdp_header.HB0 = 0;
>> +    psr_vsc.sdp_header.HB1 = 0x7;
>> +    psr_vsc.sdp_header.HB2 = 0x2;
>> +    psr_vsc.sdp_header.HB3 = 0x8;
>> +
>> +    psr_vsc.DB0 = 0;
>> +    psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | 
>> EDP_VSC_PSR_CRC_VALUES_VALID;
>> +
>> +    analogix_dp_send_psr_spd(dp, &psr_vsc);
>> +    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);
>> +    struct edp_vsc_psr psr_vsc;
>> +
>> +    if (!dp->psr_support)
>> +        return -EINVAL;
>> +
>> +    /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
>> +    memset(&psr_vsc, 0, sizeof(psr_vsc));
>> +    psr_vsc.sdp_header.HB0 = 0;
>> +    psr_vsc.sdp_header.HB1 = 0x7;
>> +    psr_vsc.sdp_header.HB2 = 0x2;
>> +    psr_vsc.sdp_header.HB3 = 0x8;
>> +
>> +    psr_vsc.DB0 = 0;
>> +    psr_vsc.DB1 = 0;
>> +
>> +    analogix_dp_send_psr_spd(dp, &psr_vsc);
>> +    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 +998,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..17fbcee 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,8 @@ 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,
>> +                  struct edp_vsc_psr *vsc);
>> +
>>   #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..52c1b6b 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -1322,3 +1322,54 @@ 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_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>> +}
>> +
>> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +                  struct edp_vsc_psr *vsc)
>> +{
>> +    unsigned int val;
>> +
>> +    /* 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 */
>> +    writel(vsc->sdp_header.HB0, dp->reg_base + ANALOGIX_DP_SPD_HB0);
>> +    writel(vsc->sdp_header.HB1, dp->reg_base + ANALOGIX_DP_SPD_HB1);
>> +    writel(vsc->sdp_header.HB2, dp->reg_base + ANALOGIX_DP_SPD_HB2);
>> +    writel(vsc->sdp_header.HB3, dp->reg_base + ANALOGIX_DP_SPD_HB3);
>> +
>> +    /* configure reused VSC PB0~PB3, magic number from vendor */
>> +    writel(0x00, dp->reg_base + ANALOGIX_DP_SPD_PB0);
>> +    writel(0x16, dp->reg_base + ANALOGIX_DP_SPD_PB1);
>> +    writel(0xCE, dp->reg_base + ANALOGIX_DP_SPD_PB2);
>> +    writel(0x5D, dp->reg_base + ANALOGIX_DP_SPD_PB3);
>> +
>> +    /* configure DB0 / DB1 values */
>> +    writel(vsc->DB0, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB0);
>> +    writel(vsc->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..40200c6 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,21 @@
>>
>>   #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_HB0            0x2F8
>> +#define ANALOGIX_DP_SPD_HB1            0x2FC
>> +#define ANALOGIX_DP_SPD_HB2            0x300
>> +#define ANALOGIX_DP_SPD_HB3            0x304
>> +#define ANALOGIX_DP_SPD_PB0            0x308
>> +#define ANALOGIX_DP_SPD_PB1            0x30C
>> +#define ANALOGIX_DP_SPD_PB2            0x310
>> +#define ANALOGIX_DP_SPD_PB3            0x314
>> +#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 +120,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 +170,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 +187,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 +402,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);
>>
>>
>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ