[PATCH 0/8] Enable eDP PSR funcionality at HSW

[PATCH 0/8] Enable eDP PSR funcionality at HSW

[Rodrigo Vivi]

PSR is an eDP feature that allows power saving even with static image at eDP screen.

Shobit Kumar has implemented this patch series some time ago, but had no eDP panel with PSR capability to test them.

I could test and verify that this series fully identify PSR capability and enables it at HSW.
I also verified that it saves from 0.5-1W but only when in blank screen. It seems it is not really entering in sleeping mode with static image at eDP screen yet.

Please accept this series as the first part of PSR enabling while we continue working to improve its functionality.

[PATCH 1/8] drm/i915: Added eDP PSR registers

[Rodrigo Vivi]

From: Shobhit Kumar <shobhit.kumar@intel.com>

v2: Add support for psr perfromance counter register and corrected
register names to reflect they are PSR registers. Also added bit
definitions for the registers

v3: Add missing CDCLK_FREQ register needed for PSR enabling.

Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 47 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5b3020f..61de724 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1561,6 +1561,40 @@
 #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
 #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
 
+/* HSW eDP PSR registers */
+#define EDP_PSR_CTL	0x64800
+#define  EDP_PSR_ENABLE		(1<<31)
+#define  EDP_PSR_LINK_DISABLE	(0<<27)
+#define  EDP_PSR_LINK_STANDBY	(1<<27)
+#define  EDP_PSR_MIN_LINK_ENTRY_TIME_MASK	(3<<25)
+#define  EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES	(0<<25)
+#define  EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES	(1<<25)
+#define  EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES	(2<<25)
+#define  EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES	(3<<25)
+#define  EDP_PSR_MAX_SLEEP_TIME_SHIFT		20
+#define  EDP_PSR_SKIP_AUX_EXIT	(1<<12)
+#define  EDP_PSR_TP1_TP2_SEL	(0<<11)
+#define  EDP_PSR_TP1_TP3_SEL	(1<<11)
+#define  EDP_PSR_TP2_TP3_TIME_500us	(0<<8)
+#define  EDP_PSR_TP2_TP3_TIME_100us	(1<<8)
+#define  EDP_PSR_TP2_TP3_TIME_2500us	(2<<8)
+#define  EDP_PSR_TP2_TP3_TIME_0us	(3<<8)
+#define  EDP_PSR_TP1_TIME_500us		(0<<4)
+#define  EDP_PSR_TP1_TIME_100us		(1<<4)
+#define  EDP_PSR_TP1_TIME_2500us	(2<<4)
+#define  EDP_PSR_TP1_TIME_0us		(3<<4)
+#define  EDP_PSR_IDLE_FRAME_SHIFT	0
+
+#define EDP_PSR_AUX_CTL	0x64810
+#define EDP_PSR_AUX_DATA1	0x64814
+#define EDP_PSR_AUX_DATA2	0x64818
+#define EDP_PSR_AUX_DATA3	0x6481c
+#define EDP_PSR_AUX_DATA4	0x64820
+#define EDP_PSR_AUX_DATA5	0x64824
+#define EDP_PSR_STATUS_CTL	0x64840
+#define  EDP_PSR_STATUS_MASK	(7<<29)
+#define EDP_PSR_PERF_CNT	0x64844
+
 /* VGA port control */
 #define ADPA			0x61100
 #define PCH_ADPA                0xe1100
@@ -1822,6 +1856,16 @@
 #define   VIDEO_DIP_ENABLE_GMP_HSW	(1 << 4)
 #define   VIDEO_DIP_ENABLE_SPD_HSW	(1 << 0)
 
+#define VIDEO_DIP_CTL_EDP	0x6F200
+ #define VIDEOP_DIP_VSC		(1<<20)
+ #define VIDEOP_DIP_GCP		(1<<16)
+ #define VIDEOP_DIP_AVI		(1<<12)
+ #define VIDEOP_DIP_VS		(1<<8)
+ #define VIDEOP_DIP_GMP		(1<<4)
+ #define VIDEOP_DIP_SPD		(1<<0)
+
+#define VIDEO_DIP_VSC_DATA_EDP	0x6F320
+
 /* Panel power sequencing */
 #define PP_STATUS	0x61200
 #define   PP_ON		(1 << 31)
@@ -3463,6 +3507,9 @@
 #define HSW_FUSE_STRAP		0x42014
 #define  HSW_CDCLK_LIMIT	(1 << 24)
 
+#define CDCLK_FREQ		0x46200
+#define CDCLK_FREQ_MASK		0x3ff
+
 /* PCH */
 
 /* south display engine interrupt: IBX */
-- 
1.7.11.7

Re: [PATCH 1/8] drm/i915: Added eDP PSR registers

[Paulo Zanoni]

Hi

2013/1/11 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v2: Add support for psr perfromance counter register and corrected
> register names to reflect they are PSR registers. Also added bit
> definitions for the registers
>
> v3: Add missing CDCLK_FREQ register needed for PSR enabling.
>
> Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 47 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5b3020f..61de724 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1561,6 +1561,40 @@
>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>
> +/* HSW eDP PSR registers */
> +#define EDP_PSR_CTL    0x64800
> +#define  EDP_PSR_ENABLE                (1<<31)
> +#define  EDP_PSR_LINK_DISABLE  (0<<27)
> +#define  EDP_PSR_LINK_STANDBY  (1<<27)
> +#define  EDP_PSR_MIN_LINK_ENTRY_TIME_MASK      (3<<25)
> +#define  EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES   (0<<25)
> +#define  EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES   (1<<25)
> +#define  EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES   (2<<25)
> +#define  EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES   (3<<25)
> +#define  EDP_PSR_MAX_SLEEP_TIME_SHIFT          20

Or you could also do something like: #define EDP_PSR_MAX_SLEEP_TIME(x)
((x) << 20)
And maybe also #define EDP_PSR_MAX_SLEEP_TIME_MASK (0x1F << 20)

> +#define  EDP_PSR_SKIP_AUX_EXIT (1<<12)
> +#define  EDP_PSR_TP1_TP2_SEL   (0<<11)
> +#define  EDP_PSR_TP1_TP3_SEL   (1<<11)
> +#define  EDP_PSR_TP2_TP3_TIME_500us    (0<<8)
> +#define  EDP_PSR_TP2_TP3_TIME_100us    (1<<8)
> +#define  EDP_PSR_TP2_TP3_TIME_2500us   (2<<8)
> +#define  EDP_PSR_TP2_TP3_TIME_0us      (3<<8)
> +#define  EDP_PSR_TP1_TIME_500us                (0<<4)
> +#define  EDP_PSR_TP1_TIME_100us                (1<<4)
> +#define  EDP_PSR_TP1_TIME_2500us       (2<<4)
> +#define  EDP_PSR_TP1_TIME_0us          (3<<4)
> +#define  EDP_PSR_IDLE_FRAME_SHIFT      0

Same thing here.

> +
> +#define EDP_PSR_AUX_CTL        0x64810
> +#define EDP_PSR_AUX_DATA1      0x64814
> +#define EDP_PSR_AUX_DATA2      0x64818
> +#define EDP_PSR_AUX_DATA3      0x6481c
> +#define EDP_PSR_AUX_DATA4      0x64820
> +#define EDP_PSR_AUX_DATA5      0x64824
> +#define EDP_PSR_STATUS_CTL     0x64840
> +#define  EDP_PSR_STATUS_MASK   (7<<29)
> +#define EDP_PSR_PERF_CNT       0x64844
> +
>  /* VGA port control */
>  #define ADPA                   0x61100
>  #define PCH_ADPA                0xe1100
> @@ -1822,6 +1856,16 @@
>  #define   VIDEO_DIP_ENABLE_GMP_HSW     (1 << 4)
>  #define   VIDEO_DIP_ENABLE_SPD_HSW     (1 << 0)
>
> +#define VIDEO_DIP_CTL_EDP      0x6F200

This is not needed. Take a look at HSW_VIDEO_DIP_CTL_A and the macro
HSW_TVIDEO_DIP_CTL. You need to make the HSW_TVIDEO_DIP_CTL macro use
cpu_transcoder instead of pipe. You can use previous patches like this
one as example:
http://lists.freedesktop.org/archives/intel-gfx/2012-October/020907.html

> + #define VIDEOP_DIP_VSC                (1<<20)

See VIDEO_DIP_ENABLE_VSC_HSW.

> + #define VIDEOP_DIP_GCP                (1<<16)

See VIDEO_DIP_ENABLE_GCP_HSW.

> + #define VIDEOP_DIP_AVI                (1<<12)

See VIDEO_DIP_ENABLE_AVI_HSW.

> + #define VIDEOP_DIP_VS         (1<<8)

See VIDEO_DIP_ENABLE_VS_HSW.

> + #define VIDEOP_DIP_GMP                (1<<4)

See VIDEO_DIP_ENABLE_GMP_HSW.

> + #define VIDEOP_DIP_SPD                (1<<0)

See VIDEO_DIP_ENABLE_SPD_HSW.

> +
> +#define VIDEO_DIP_VSC_DATA_EDP 0x6F320
> +

Take a look at the definition of HSW_TVIDEO_DIP_CTL and similar
macros. You'lll probably have to port them to use cpu_transcoder
instead of pipe too.

>  /* Panel power sequencing */
>  #define PP_STATUS      0x61200
>  #define   PP_ON                (1 << 31)
> @@ -3463,6 +3507,9 @@
>  #define HSW_FUSE_STRAP         0x42014
>  #define  HSW_CDCLK_LIMIT       (1 << 24)
>
> +#define CDCLK_FREQ             0x46200
> +#define CDCLK_FREQ_MASK                0x3ff

Last time I checked, these registers should not be used. Please see
intel_ddi_get_cdclk_freq() from intel_ddi.c.

> +
>  /* PCH */
>
>  /* south display engine interrupt: IBX */
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

[PATCH 2/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP

[Rodrigo Vivi]

From: Shobhit Kumar <shobhit.kumar@intel.com>

Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>

v2: Modified and corrected the structures to be more in line for
kernel coding guidelines and rebased the code on Paulo's DP patchset

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 116580b..9799fe9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -371,10 +371,38 @@ struct intel_dp {
 	int backlight_on_delay;
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
+	uint8_t psr_setup;
 	bool want_panel_vdd;
 	struct intel_connector *attached_connector;
 };
 
+/* SDP header as per eDP 1.3 spec, section 3.6 */
+struct edp_sdp_header {
+	u8 id;
+	u8 type;
+	u8 revision : 5;   	    /* Bits 0:4 */
+	u8 rsvd1    : 3;   	    /* Bits 5:7 */
+	u8 valid_payload_bytes : 5; /* Bits 0:4 */
+	u8 rsvd2	       : 3; /* Bits 5:7 */
+} __attribute__((packed));
+
+/* SDP VSC header as per eDP 1.3 spec, section 3.6 */
+struct edp_vsc_psr {
+	struct edp_sdp_header sdp_header;
+	u8 unused;
+	u8 psr_state  : 1; 	/* Bit 0 */
+	u8 update_rfb : 1;	/* Bit 1 */
+	u8 valid_crc  : 1;	/* Bit 2 */
+	u8 reserved1  : 5;	/* Bits 3:7 */
+	u8 crc_r_lower;
+	u8 crc_r_higher;
+	u8 crc_g_lower;
+	u8 crc_g_higher;
+	u8 crc_b_lower;
+	u8 crc_b_higher;
+	u8 reserved2[24];
+} __attribute__((packed));
+
 struct intel_digital_port {
 	struct intel_encoder base;
 	enum port port;
-- 
1.7.11.7

Re: [PATCH 2/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP

[Daniel Vetter]

On Fri, Jan 11, 2013 at 05:57:51PM -0200, Rodrigo Vivi wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> 
> v2: Modified and corrected the structures to be more in line for
> kernel coding guidelines and rebased the code on Paulo's DP patchset
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

DP spec stuff should be put into the drm core imo, i.e. into
include/drm/drm_dp_helper.h Since all bitfields are u8 we also have no
problems with endianess for this.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 116580b..9799fe9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -371,10 +371,38 @@ struct intel_dp {
>  	int backlight_on_delay;
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
> +	uint8_t psr_setup;
>  	bool want_panel_vdd;
>  	struct intel_connector *attached_connector;
>  };
>  
> +/* SDP header as per eDP 1.3 spec, section 3.6 */
> +struct edp_sdp_header {
> +	u8 id;
> +	u8 type;
> +	u8 revision : 5;   	    /* Bits 0:4 */
> +	u8 rsvd1    : 3;   	    /* Bits 5:7 */
> +	u8 valid_payload_bytes : 5; /* Bits 0:4 */
> +	u8 rsvd2	       : 3; /* Bits 5:7 */
> +} __attribute__((packed));
> +
> +/* SDP VSC header as per eDP 1.3 spec, section 3.6 */
> +struct edp_vsc_psr {
> +	struct edp_sdp_header sdp_header;
> +	u8 unused;
> +	u8 psr_state  : 1; 	/* Bit 0 */
> +	u8 update_rfb : 1;	/* Bit 1 */
> +	u8 valid_crc  : 1;	/* Bit 2 */
> +	u8 reserved1  : 5;	/* Bits 3:7 */
> +	u8 crc_r_lower;
> +	u8 crc_r_higher;
> +	u8 crc_g_lower;
> +	u8 crc_g_higher;
> +	u8 crc_b_lower;
> +	u8 crc_b_higher;
> +	u8 reserved2[24];
> +} __attribute__((packed));
> +
>  struct intel_digital_port {
>  	struct intel_encoder base;
>  	enum port port;
> -- 
> 1.7.11.7
> 
> _______________________________________________
> 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

Re: [PATCH 2/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP

[Ville Syrjälä]

On Fri, Jan 11, 2013 at 09:15:40PM +0100, Daniel Vetter wrote:
> On Fri, Jan 11, 2013 at 05:57:51PM -0200, Rodrigo Vivi wrote:
> > From: Shobhit Kumar <shobhit.kumar@intel.com>
> > 
> > Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> > 
> > v2: Modified and corrected the structures to be more in line for
> > kernel coding guidelines and rebased the code on Paulo's DP patchset
> > 
> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> > 
> > v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> DP spec stuff should be put into the drm core imo, i.e. into
> include/drm/drm_dp_helper.h Since all bitfields are u8 we also have no
> problems with endianess for this.

I don't think C guarantees anything about the order of bits inside
bitfieds.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 116580b..9799fe9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -371,10 +371,38 @@ struct intel_dp {
> >  	int backlight_on_delay;
> >  	int backlight_off_delay;
> >  	struct delayed_work panel_vdd_work;
> > +	uint8_t psr_setup;
> >  	bool want_panel_vdd;
> >  	struct intel_connector *attached_connector;
> >  };
> >  
> > +/* SDP header as per eDP 1.3 spec, section 3.6 */
> > +struct edp_sdp_header {
> > +	u8 id;
> > +	u8 type;
> > +	u8 revision : 5;   	    /* Bits 0:4 */
> > +	u8 rsvd1    : 3;   	    /* Bits 5:7 */
> > +	u8 valid_payload_bytes : 5; /* Bits 0:4 */
> > +	u8 rsvd2	       : 3; /* Bits 5:7 */
> > +} __attribute__((packed));
> > +
> > +/* SDP VSC header as per eDP 1.3 spec, section 3.6 */
> > +struct edp_vsc_psr {
> > +	struct edp_sdp_header sdp_header;
> > +	u8 unused;
> > +	u8 psr_state  : 1; 	/* Bit 0 */
> > +	u8 update_rfb : 1;	/* Bit 1 */
> > +	u8 valid_crc  : 1;	/* Bit 2 */
> > +	u8 reserved1  : 5;	/* Bits 3:7 */
> > +	u8 crc_r_lower;
> > +	u8 crc_r_higher;
> > +	u8 crc_g_lower;
> > +	u8 crc_g_higher;
> > +	u8 crc_b_lower;
> > +	u8 crc_b_higher;
> > +	u8 reserved2[24];
> > +} __attribute__((packed));
> > +
> >  struct intel_digital_port {
> >  	struct intel_encoder base;
> >  	enum port port;
> > -- 
> > 1.7.11.7
> > 
> > _______________________________________________
> > 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP

[Philipp Klaus Krause]

Am 14.01.2013 12:22, schrieb Ville Syrjälä:
> On Fri, Jan 11, 2013 at 09:15:40PM +0100, Daniel Vetter wrote:
>> On Fri, Jan 11, 2013 at 05:57:51PM -0200, Rodrigo Vivi wrote:
>>> From: Shobhit Kumar <shobhit.kumar@intel.com>
>>>
>>> Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
>>>
>>> v2: Modified and corrected the structures to be more in line for
>>> kernel coding guidelines and rebased the code on Paulo's DP patchset
>>>
>>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>>>
>>> v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
>>>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>>
>> DP spec stuff should be put into the drm core imo, i.e. into
>> include/drm/drm_dp_helper.h Since all bitfields are u8 we also have no
>> problems with endianess for this.
> 
> I don't think C guarantees anything about the order of bits inside
> bitfieds.
> 

C11 standard, section 6.7.2.1:

"An implementation may allocate any addressable storage unit large
enough to hold a bit-field. If enough space remains, a bit-field that
immediately follows another bit-field in a structure shall be packed
into adjacent bits of the same unit. If insufficient space remains,
whether a bit-field that does not fit is put into the next unit or
overlaps adjacent units is implementation-defined. The order of
allocation of bit-fields within a unit (high-order to low-order or
low-order to high-order) is implementation-defined. The alignment of the
addressable storage unit is unspecified."

C doesn't guarantee that your u8 is a valid type for bit-fields. The
only types a C compiler must support for bit-fields are bool, int,
signed int and unsigned int (note that bit-fields are also the only
place where the type int can be different from signed int).

Philipp

[PATCH 3/8] drm/i915: Read the EDP DPCD and PSR Capability

[Rodrigo Vivi]

From: Shobhit Kumar <shobhit.kumar@intel.com>

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v2: reuse of just created is_edp_psr and put it at right place.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 include/drm/drm_dp_helper.h      |  1 +
 3 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1dd89d5..f0224f8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1418,6 +1418,11 @@ static void intel_post_disable_dp(struct intel_encoder *encoder)
 	}
 }
 
+static bool is_edp_psr(struct intel_dp *intel_dp)
+{
+	return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));
+}
+
 static void intel_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -2094,6 +2099,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
 		return false; /* DPCD not present */
 
+	/* Check if the panel supports PSR */
+	memset(intel_dp->psr_dpcd, 0, EDP_PSR_RECEIVER_CAP_SIZE);
+	intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
+				       intel_dp->psr_dpcd,
+				       sizeof(intel_dp->psr_dpcd));
+	if (is_edp_psr(intel_dp))
+		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
 	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
 	      DP_DWN_STRM_PORT_PRESENT))
 		return true; /* native DP sink */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9799fe9..82a85ad 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -360,6 +360,7 @@ struct intel_dp {
 	uint8_t link_bw;
 	uint8_t lane_count;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	struct i2c_adapter adapter;
 	struct i2c_algo_dp_aux_data algo;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index c09d367..32eeb92 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -335,6 +335,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
 					  int lane);
 
 #define DP_RECEIVER_CAP_SIZE	0xf
+#define EDP_PSR_RECEIVER_CAP_SIZE	2
 void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 
-- 
1.7.11.7

Re: [PATCH 3/8] drm/i915: Read the EDP DPCD and PSR Capability

[Paulo Zanoni]

Hi

2013/1/11 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v2: reuse of just created is_edp_psr and put it at right place.
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  include/drm/drm_dp_helper.h      |  1 +
>  3 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1dd89d5..f0224f8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1418,6 +1418,11 @@ static void intel_post_disable_dp(struct intel_encoder *encoder)
>         }
>  }
>
> +static bool is_edp_psr(struct intel_dp *intel_dp)
> +{
> +       return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));

Bikeshedding:
Since the spec of address 0x70 says "All other values are reserved for
future versions of PSR", maybe we could do something like "return
(is_edp(intel_dp) && !(intel_dp->psr_dpcd[0] == 0)" ?

> +}
> +
>  static void intel_enable_dp(struct intel_encoder *encoder)
>  {
>         struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2094,6 +2099,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>         if (intel_dp->dpcd[DP_DPCD_REV] == 0)
>                 return false; /* DPCD not present */
>
> +       /* Check if the panel supports PSR */
> +       memset(intel_dp->psr_dpcd, 0, EDP_PSR_RECEIVER_CAP_SIZE);
> +       intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
> +                                      intel_dp->psr_dpcd,
> +                                      sizeof(intel_dp->psr_dpcd));
> +       if (is_edp_psr(intel_dp))
> +               DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
>         if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>               DP_DWN_STRM_PORT_PRESENT))
>                 return true; /* native DP sink */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9799fe9..82a85ad 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -360,6 +360,7 @@ struct intel_dp {
>         uint8_t link_bw;
>         uint8_t lane_count;
>         uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> +       uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>         uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>         struct i2c_adapter adapter;
>         struct i2c_algo_dp_aux_data algo;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index c09d367..32eeb92 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -335,6 +335,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
>                                           int lane);
>
>  #define DP_RECEIVER_CAP_SIZE   0xf
> +#define EDP_PSR_RECEIVER_CAP_SIZE      2
>  void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
>  void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
>
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

[PATCH 4/8] drm/i915: Setup EDP PSR AUX Registers

[Rodrigo Vivi]

From: Shobhit Kumar <shobhit.kumar@intel.com>

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 49 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f0224f8..c35caf1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1423,6 +1423,55 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
 	return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));
 }
 
+static void intel_edp_psr_setup(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t aux_clock_divider;
+	int precharge = 0x3;
+	int msg_size = 5;	/* Header(4) + Message(1) */
+
+	/* No need to setup if already done as these setting are persistent
+	 * until power states are entered */
+	if (intel_dp->psr_setup)
+		return;
+
+	/* Setup AUX registers */
+	/* Write command on DPCD 0x0600 */
+	I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
+
+	/* Set the state to normal operation D0 in DPCD 0x0600 */
+	I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);
+
+	if (is_cpu_edp(intel_dp)) {
+		if (!IS_HASWELL(dev) && (IS_GEN6(dev) || IS_GEN7(dev)))
+			aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */
+		else
+			/* CDCLK divided by 2 */
+			aux_clock_divider = ((I915_READ(CDCLK_FREQ) & CDCLK_FREQ_MASK) + 1)/2;
+	} else if (HAS_PCH_SPLIT(dev))
+		aux_clock_divider = 63; /* IRL input clock fixed at 125Mhz */
+	else
+		aux_clock_divider = intel_hrawclk(dev) / 2;
+
+	I915_WRITE(EDP_PSR_AUX_CTL,
+		   DP_AUX_CH_CTL_TIME_OUT_400us |
+		   (msg_size << 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));
+
+	/* Setup the debug register */
+	I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) |
+			EDP_PSR_DEBUG_MASK_MEMUP |
+			EDP_PSR_DEBUG_MASK_HPD);
+
+	/* This flag can be made to 0 from pm code so as to reinitialize the
+	 * AUX register in case of power states, returning from which will not
+	 * maintain the AUX register settings
+	 */
+	intel_dp->psr_setup = 1;
+}
+
 static void intel_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-- 
1.7.11.7

Re: [PATCH 4/8] drm/i915: Setup EDP PSR AUX Registers

[Paulo Zanoni]

Hi

2013/1/11 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 49 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f0224f8..c35caf1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1423,6 +1423,55 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>         return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));
>  }
>
> +static void intel_edp_psr_setup(struct intel_dp *intel_dp)
> +{
> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       uint32_t aux_clock_divider;
> +       int precharge = 0x3;
> +       int msg_size = 5;       /* Header(4) + Message(1) */
> +
> +       /* No need to setup if already done as these setting are persistent
> +        * until power states are entered */
> +       if (intel_dp->psr_setup)
> +               return;
> +
> +       /* Setup AUX registers */
> +       /* Write command on DPCD 0x0600 */
> +       I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
> +
> +       /* Set the state to normal operation D0 in DPCD 0x0600 */
> +       I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);
> +
> +       if (is_cpu_edp(intel_dp)) {
> +               if (!IS_HASWELL(dev) && (IS_GEN6(dev) || IS_GEN7(dev)))
> +                       aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */
> +               else
> +                       /* CDCLK divided by 2 */
> +                       aux_clock_divider = ((I915_READ(CDCLK_FREQ) & CDCLK_FREQ_MASK) + 1)/2;
> +       } else if (HAS_PCH_SPLIT(dev))
> +               aux_clock_divider = 63; /* IRL input clock fixed at 125Mhz */
> +       else
> +               aux_clock_divider = intel_hrawclk(dev) / 2;
> +
> +       I915_WRITE(EDP_PSR_AUX_CTL,
> +                  DP_AUX_CH_CTL_TIME_OUT_400us |
> +                  (msg_size << 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));
> +
> +       /* Setup the debug register */
> +       I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) |
> +                       EDP_PSR_DEBUG_MASK_MEMUP |
> +                       EDP_PSR_DEBUG_MASK_HPD);
> +
> +       /* This flag can be made to 0 from pm code so as to reinitialize the
> +        * AUX register in case of power states, returning from which will not
> +        * maintain the AUX register settings
> +        */
> +       intel_dp->psr_setup = 1;
> +}
> +

Looks like this function has a lot in common with intel_dp_aux_ch. Can
we try to reuse its code? Maybe we could patch intel_dp_aux_ch to
support using the SRD registers instead of the standard registers?
Also we should probably create "intel_dp_get_aux_clock_divider()".

>  static void intel_enable_dp(struct intel_encoder *encoder)
>  {
>         struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

[PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW

[Rodrigo Vivi]

From: Shobhit Kumar <shobhit.kumar@intel.com>

Parse and store useful information in i915_dev_private

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  8 ++++++++
 drivers/gpu/drm/i915/intel_bios.c | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_bios.h | 20 +++++++++++++++++++-
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d2b93a4..6cb3439 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -751,6 +751,14 @@ typedef struct drm_i915_private {
 	} edp;
 	bool no_aux_handshake;
 
+
+	/* PSR related info from VBT */
+	u8 full_link_state;
+	u8 wait_lines;
+	u8 idle_frames;
+	u16 wakeup_tp1;
+	u16 wakeup_tp2_tp3;
+
 	int crt_ddc_pin;
 	struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume 965 */
 	int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 87e9b92..a5fa3c4 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -381,6 +381,35 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
 	}
 }
 
+
+static void
+parse_edp_psr(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
+{
+	struct bdb_psr_features *psr;
+	struct bdb_lvds_options *lvds_opts;
+	int index = 0;
+	lvds_opts = find_section(bdb, BDB_LVDS_OPTIONS);
+	if (!lvds_opts) {
+		DRM_DEBUG_KMS("No LVDS Options block found.\n");
+		return;
+	}
+
+	index = lvds_opts->panel_type;
+
+	psr = find_section(bdb, BDB_PSR_FEATURES);
+	if (!psr) {
+		DRM_DEBUG_KMS("No PSR feature block found.\n");
+		return;
+	}
+
+	dev_priv->full_link_state = psr[index].link_disable;
+	dev_priv->wait_lines = psr[index].wait_lines;
+	dev_priv->idle_frames = psr[index].idle_frames;
+	dev_priv->wakeup_tp1 = psr[index].wakeup_tp1;
+	dev_priv->wakeup_tp2_tp3 = psr[index].wakeup_tp2;
+}
+
+
 static void
 parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 			  struct bdb_header *bdb)
@@ -747,6 +776,7 @@ intel_parse_bios(struct drm_device *dev)
 	parse_device_mapping(dev_priv, bdb);
 	parse_driver_features(dev_priv, bdb);
 	parse_edp(dev_priv, bdb);
+	parse_edp_psr(dev_priv, bdb);
 
 	if (bios)
 		pci_unmap_rom(pdev, bios);
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 36e57f9..c1d39de 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -80,7 +80,7 @@ struct vbios_data {
 #define BDB_EXT_MMIO_REGS	  6
 #define BDB_SWF_IO		  7
 #define BDB_SWF_MMIO		  8
-#define BDB_DOT_CLOCK_TABLE	  9
+#define BDB_PSR_FEATURES	  9
 #define BDB_MODE_REMOVAL_TABLE	 10
 #define BDB_CHILD_DEVICE_TABLE	 11
 #define BDB_DRIVER_FEATURES	 12
@@ -263,6 +263,24 @@ struct bdb_lvds_options {
 	u8 rsvd4;
 } __attribute__((packed));
 
+struct bdb_psr_features {
+	/* psr_enable byte */
+	u8 link_disable:1;
+	u8 require_aux:1;
+	u8 rsvd1:6;
+
+	/* panel wait times */
+	u8 idle_frames:4;
+	u8 wait_lines:3;
+	u8 rsvd2:1;
+
+	/* TP1 wakeup time */
+	u16 wakeup_tp1;
+
+	/* TP2 wakeup time */
+	u16 wakeup_tp2;
+} __attribute__((packed));
+
 /* LFP pointer table contains entries to the struct below */
 struct bdb_lvds_lfp_data_ptr {
 	u16 fp_timing_offset; /* offsets are from start of bdb */
-- 
1.7.11.7

Re: [PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW

[Daniel Vetter]

On Fri, Jan 11, 2013 at 05:57:54PM -0200, Rodrigo Vivi wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> Parse and store useful information in i915_dev_private
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  8 ++++++++
>  drivers/gpu/drm/i915/intel_bios.c | 30 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_bios.h | 20 +++++++++++++++++++-
>  3 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d2b93a4..6cb3439 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -751,6 +751,14 @@ typedef struct drm_i915_private {
>  	} edp;
>  	bool no_aux_handshake;
>  
> +
> +	/* PSR related info from VBT */
> +	u8 full_link_state;
> +	u8 wait_lines;
> +	u8 idle_frames;
> +	u16 wakeup_tp1;
> +	u16 wakeup_tp2_tp3;

Recently started holy war of mine: Can we pls now shovel random piles of
things into dev_priv? Preferred would be if only intel_dp.c would need to
care about this, otherwise a meaningful substruct (with the struct
definition out of line) is the 2nd best option ...
-Daniel

> +
>  	int crt_ddc_pin;
>  	struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume 965 */
>  	int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 87e9b92..a5fa3c4 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -381,6 +381,35 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +
> +static void
> +parse_edp_psr(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
> +{
> +	struct bdb_psr_features *psr;
> +	struct bdb_lvds_options *lvds_opts;
> +	int index = 0;
> +	lvds_opts = find_section(bdb, BDB_LVDS_OPTIONS);
> +	if (!lvds_opts) {
> +		DRM_DEBUG_KMS("No LVDS Options block found.\n");
> +		return;
> +	}
> +
> +	index = lvds_opts->panel_type;
> +
> +	psr = find_section(bdb, BDB_PSR_FEATURES);
> +	if (!psr) {
> +		DRM_DEBUG_KMS("No PSR feature block found.\n");
> +		return;
> +	}
> +
> +	dev_priv->full_link_state = psr[index].link_disable;
> +	dev_priv->wait_lines = psr[index].wait_lines;
> +	dev_priv->idle_frames = psr[index].idle_frames;
> +	dev_priv->wakeup_tp1 = psr[index].wakeup_tp1;
> +	dev_priv->wakeup_tp2_tp3 = psr[index].wakeup_tp2;
> +}
> +
> +
>  static void
>  parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>  			  struct bdb_header *bdb)
> @@ -747,6 +776,7 @@ intel_parse_bios(struct drm_device *dev)
>  	parse_device_mapping(dev_priv, bdb);
>  	parse_driver_features(dev_priv, bdb);
>  	parse_edp(dev_priv, bdb);
> +	parse_edp_psr(dev_priv, bdb);
>  
>  	if (bios)
>  		pci_unmap_rom(pdev, bios);
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 36e57f9..c1d39de 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -80,7 +80,7 @@ struct vbios_data {
>  #define BDB_EXT_MMIO_REGS	  6
>  #define BDB_SWF_IO		  7
>  #define BDB_SWF_MMIO		  8
> -#define BDB_DOT_CLOCK_TABLE	  9
> +#define BDB_PSR_FEATURES	  9
>  #define BDB_MODE_REMOVAL_TABLE	 10
>  #define BDB_CHILD_DEVICE_TABLE	 11
>  #define BDB_DRIVER_FEATURES	 12
> @@ -263,6 +263,24 @@ struct bdb_lvds_options {
>  	u8 rsvd4;
>  } __attribute__((packed));
>  
> +struct bdb_psr_features {
> +	/* psr_enable byte */
> +	u8 link_disable:1;
> +	u8 require_aux:1;
> +	u8 rsvd1:6;
> +
> +	/* panel wait times */
> +	u8 idle_frames:4;
> +	u8 wait_lines:3;
> +	u8 rsvd2:1;
> +
> +	/* TP1 wakeup time */
> +	u16 wakeup_tp1;
> +
> +	/* TP2 wakeup time */
> +	u16 wakeup_tp2;
> +} __attribute__((packed));
> +
>  /* LFP pointer table contains entries to the struct below */
>  struct bdb_lvds_lfp_data_ptr {
>  	u16 fp_timing_offset; /* offsets are from start of bdb */
> -- 
> 1.7.11.7
> 
> _______________________________________________
> 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

Re: [PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW

[Rodrigo Vivi]

Actually these things comes from vbt.

Maybe we could create a vbt struct to place things like this and only
this vbt struct inside dev_priv..
What do you think?


On Fri, Jan 11, 2013 at 6:46 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Jan 11, 2013 at 05:57:54PM -0200, Rodrigo Vivi wrote:
>> From: Shobhit Kumar <shobhit.kumar@intel.com>
>>
>> Parse and store useful information in i915_dev_private
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  8 ++++++++
>>  drivers/gpu/drm/i915/intel_bios.c | 30 ++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_bios.h | 20 +++++++++++++++++++-
>>  3 files changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index d2b93a4..6cb3439 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -751,6 +751,14 @@ typedef struct drm_i915_private {
>>       } edp;
>>       bool no_aux_handshake;
>>
>> +
>> +     /* PSR related info from VBT */
>> +     u8 full_link_state;
>> +     u8 wait_lines;
>> +     u8 idle_frames;
>> +     u16 wakeup_tp1;
>> +     u16 wakeup_tp2_tp3;
>
> Recently started holy war of mine: Can we pls now shovel random piles of
> things into dev_priv? Preferred would be if only intel_dp.c would need to
> care about this, otherwise a meaningful substruct (with the struct
> definition out of line) is the 2nd best option ...
> -Daniel
>
>> +
>>       int crt_ddc_pin;
>>       struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume 965 */
>>       int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 87e9b92..a5fa3c4 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -381,6 +381,35 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
>>       }
>>  }
>>
>> +
>> +static void
>> +parse_edp_psr(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>> +{
>> +     struct bdb_psr_features *psr;
>> +     struct bdb_lvds_options *lvds_opts;
>> +     int index = 0;
>> +     lvds_opts = find_section(bdb, BDB_LVDS_OPTIONS);
>> +     if (!lvds_opts) {
>> +             DRM_DEBUG_KMS("No LVDS Options block found.\n");
>> +             return;
>> +     }
>> +
>> +     index = lvds_opts->panel_type;
>> +
>> +     psr = find_section(bdb, BDB_PSR_FEATURES);
>> +     if (!psr) {
>> +             DRM_DEBUG_KMS("No PSR feature block found.\n");
>> +             return;
>> +     }
>> +
>> +     dev_priv->full_link_state = psr[index].link_disable;
>> +     dev_priv->wait_lines = psr[index].wait_lines;
>> +     dev_priv->idle_frames = psr[index].idle_frames;
>> +     dev_priv->wakeup_tp1 = psr[index].wakeup_tp1;
>> +     dev_priv->wakeup_tp2_tp3 = psr[index].wakeup_tp2;
>> +}
>> +
>> +
>>  static void
>>  parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>>                         struct bdb_header *bdb)
>> @@ -747,6 +776,7 @@ intel_parse_bios(struct drm_device *dev)
>>       parse_device_mapping(dev_priv, bdb);
>>       parse_driver_features(dev_priv, bdb);
>>       parse_edp(dev_priv, bdb);
>> +     parse_edp_psr(dev_priv, bdb);
>>
>>       if (bios)
>>               pci_unmap_rom(pdev, bios);
>> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
>> index 36e57f9..c1d39de 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>> @@ -80,7 +80,7 @@ struct vbios_data {
>>  #define BDB_EXT_MMIO_REGS      6
>>  #define BDB_SWF_IO             7
>>  #define BDB_SWF_MMIO           8
>> -#define BDB_DOT_CLOCK_TABLE    9
>> +#define BDB_PSR_FEATURES       9
>>  #define BDB_MODE_REMOVAL_TABLE        10
>>  #define BDB_CHILD_DEVICE_TABLE        11
>>  #define BDB_DRIVER_FEATURES   12
>> @@ -263,6 +263,24 @@ struct bdb_lvds_options {
>>       u8 rsvd4;
>>  } __attribute__((packed));
>>
>> +struct bdb_psr_features {
>> +     /* psr_enable byte */
>> +     u8 link_disable:1;
>> +     u8 require_aux:1;
>> +     u8 rsvd1:6;
>> +
>> +     /* panel wait times */
>> +     u8 idle_frames:4;
>> +     u8 wait_lines:3;
>> +     u8 rsvd2:1;
>> +
>> +     /* TP1 wakeup time */
>> +     u16 wakeup_tp1;
>> +
>> +     /* TP2 wakeup time */
>> +     u16 wakeup_tp2;
>> +} __attribute__((packed));
>> +
>>  /* LFP pointer table contains entries to the struct below */
>>  struct bdb_lvds_lfp_data_ptr {
>>       u16 fp_timing_offset; /* offsets are from start of bdb */
>> --
>> 1.7.11.7
>>
>> _______________________________________________
>> 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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW

[Daniel Vetter]

On Tue, Jan 15, 2013 at 2:23 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> Actually these things comes from vbt.
>
> Maybe we could create a vbt struct to place things like this and only
> this vbt struct inside dev_priv..
> What do you think?

Yeah, a vbt substruct with parsed stuff seems the sanest option.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 6/8] drm/i915: Enable/Disable PSR on HSW

[Rodrigo Vivi]

From: Shobhit Kumar <shobhit.kumar@intel.com>

Added eDP PSR enable functionality. This includes setting the PSR
configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
enabling PSR in the sink via DPCD register and finally enabling PSR on
the host. PSR works only in LPSP mode, so put the PIPE_DDI in DDIA
always on

This patch is heavily based on initial PSR code by Sateesh Kavuri but is
quite different in implementation. Makes use of VBT parsed data and also
the code has been cleaned up.

Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v2: fix getting base.crtc from intel_dp and fix DDI_EDP_INPUT_A_ON entry
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |   3 +
 drivers/gpu/drm/i915/intel_ddi.c |   6 +-
 drivers/gpu/drm/i915/intel_dp.c  | 170 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 4 files changed, 181 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 61de724..289c6c0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1594,6 +1594,9 @@
 #define EDP_PSR_STATUS_CTL	0x64840
 #define  EDP_PSR_STATUS_MASK	(7<<29)
 #define EDP_PSR_PERF_CNT	0x64844
+#define EDP_PSR_DEBUG_CTL	0x64860
+#define  EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
+#define  EDP_PSR_DEBUG_MASK_HPD		(1<<25)
 
 /* VGA port control */
 #define ADPA			0x61100
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f02b3fe..05d3a30 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -958,9 +958,13 @@ void intel_ddi_enable_pipe_func(struct drm_crtc *crtc)
 		temp |= TRANS_DDI_PHSYNC;
 
 	if (cpu_transcoder == TRANSCODER_EDP) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		switch (pipe) {
 		case PIPE_A:
-			temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
+			if (intel_dp->psr_dpcd[0] & 0x1)
+				temp |= TRANS_DDI_EDP_INPUT_A_ON;
+			else
+				temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
 			break;
 		case PIPE_B:
 			temp |= TRANS_DDI_EDP_INPUT_B_ONOFF;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c35caf1..208128c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -83,6 +83,13 @@ static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
 	return intel_dig_port->base.base.dev;
 }
 
+static struct drm_crtc *intel_dp_to_crtc(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+
+	return intel_dig_port->base.base.crtc;
+}
+
 static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 {
 	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
@@ -1472,6 +1479,169 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
 	intel_dp->psr_setup = 1;
 }
 
+static bool
+intel_edp_is_psr_enabled(struct intel_dp* intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	return (I915_READ(EDP_PSR_CTL) & (1<<31)) ? true : false;
+}
+
+
+static void
+intel_edp_psr_enable_src(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t max_sleep_time = 0x1f;
+	uint32_t val = 0x0;
+
+	/* Use VBT values which are parsed in
+	 * dev_priv->idle_frames,
+	 * but the BIOS initializes this to zero today
+	 * so hardcode
+	 */
+	uint32_t idle_frames = 6;
+
+	if (intel_dp->psr_dpcd[1] & 0x1) {
+		/* No link training on PSR Exit required */
+		val |= EDP_PSR_TP2_TP3_TIME_0us;
+		val |= EDP_PSR_TP1_TIME_0us;
+		val |= EDP_PSR_SKIP_AUX_EXIT;
+	} else {
+		/* Use these Values from VBT
+		 * Case values are timings for HSW as of now
+		 * in multiple of 100us
+		 */
+		switch(dev_priv->wakeup_tp1) {
+			case 1:
+				val |= EDP_PSR_TP1_TIME_100us;
+				break;
+			case 5:
+				val |= EDP_PSR_TP1_TIME_500us;
+				break;
+			case 25:
+				val |= EDP_PSR_TP1_TIME_2500us;
+				break;
+			default:
+				val |= EDP_PSR_TP1_TIME_500us;
+				break;
+		};
+		switch(dev_priv->wakeup_tp2_tp3) {
+			case 1:
+				val |= EDP_PSR_TP2_TP3_TIME_100us;
+				break;
+			case 5:
+				val |= EDP_PSR_TP2_TP3_TIME_500us;
+				break;
+			case 25:
+				val |= EDP_PSR_TP2_TP3_TIME_2500us;
+				break;
+			default:
+				val |= EDP_PSR_TP2_TP3_TIME_500us;
+				break;
+		};
+	}
+
+	/* Disable main link. Anyway in HSW steppings today
+	 * link standby does not work
+	 *
+	 * Later used VBT info (already parsed and available)
+	 * while supporting standby we need to program
+	 * val |= EDP_PSR_MIN_LINK_ENTRY_TIME_X_LINES based on VBT
+	 */
+	val = (val & ~EDP_PSR_LINK_STANDBY) |
+		(max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT) |
+		(idle_frames << EDP_PSR_IDLE_FRAME_SHIFT) |
+		EDP_PSR_ENABLE;
+
+	I915_WRITE(EDP_PSR_CTL, val);
+}
+
+void intel_edp_enable_psr(struct intel_dp* intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
+	struct edp_vsc_psr psr_vsc;
+	uint32_t *vsc_data = (uint32_t *) &psr_vsc;
+	int i = 0, vsc_len = sizeof(struct edp_vsc_psr);
+
+	if (!is_edp_psr(intel_dp))
+		return;
+
+	/* setup AUX registers in case returned from pm states */
+	intel_edp_psr_setup(intel_dp);
+
+	/* Check if PSR is already enabled */
+	if (!intel_edp_is_psr_enabled(intel_dp)) {
+		/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
+		memset(&psr_vsc, 0, sizeof(psr_vsc));
+		psr_vsc.sdp_header.id = 0;
+		psr_vsc.sdp_header.type = 0x7;
+		psr_vsc.sdp_header.revision = 0x2;
+		psr_vsc.sdp_header.valid_payload_bytes = 0x8;
+
+		/* As per eDP spec, wait for vblank to send SDP VSC packet */
+		intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+		/* Load the VSC DIP packet */
+		for(i = 0; i < vsc_len; i += 4)
+			I915_WRITE((VIDEO_DIP_VSC_DATA_EDP + i), vsc_data[i]);
+#if 0
+		/* TBD:
+		 * We might not have to do explicitely as hardware will take care of this */
+		/* Enable the DIP register */
+		val = I915_READ(VIDEO_DIP_CTL_EDP);
+		I915_WRITE(VIDEO_DIP_CTL_EDP, val | VIDEOP_DIP_VSC);
+#endif
+		/* Enable PSR in sink by setting bit 0 in DPCD config reg
+		 * along with the transmitter state during PSR active
+		 * Transmitter state later can be ready from VBT. As of now
+		 * program the full link down
+		 *
+		 */
+		intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
+						DP_PSR_ENABLE &
+						~DP_PSR_MAIN_LINK_ACTIVE);
+
+		/* Enable PSR on the host */
+		intel_edp_psr_enable_src(intel_dp);
+	}
+}
+
+void intel_edp_disable_psr(struct intel_dp* intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
+	uint32_t val;
+	if (!intel_edp_is_psr_enabled(intel_dp))
+		return;
+
+	val = I915_READ(EDP_PSR_CTL);
+	I915_WRITE(EDP_PSR_CTL, (val & ~EDP_PSR_ENABLE));
+
+	/* Wait till PSR is idle */
+	if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) & EDP_PSR_STATUS_MASK) == 0, 2000, 10))
+		DRM_ERROR("Timed out waiting for PSR Idle State\n");
+
+	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+#if 0
+	/* TBD:
+	 * Following is not yet confirmed from H/W team.
+	 * As per last discussion we do not need to disable
+	 * VSC DIP explicitely. Just maintaining the code in
+	 * case we have to do this later at some point
+	 */
+
+	/* Disable VSC DIP */
+	val = I915_READ(VIDEO_DIP_CTL_EDP);
+	I915_WRITE(VIDEO_DIP_CTL_EDP, val & ~VIDEOP_DIP_VSC);
+#endif
+}
+
 static void intel_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 82a85ad..a9960be 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -704,4 +704,7 @@ extern bool
 intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
 extern void intel_ddi_fdi_disable(struct drm_crtc *crtc);
 
+extern void intel_edp_enable_psr(struct intel_dp* intel_dp);
+extern void intel_edp_disable_psr(struct intel_dp* intel_dp);
+
 #endif /* __INTEL_DRV_H__ */
-- 
1.7.11.7

[PATCH 7/8] drm/i915: Added debugfs support for PSR Status

[Rodrigo Vivi]

From: Shobhit Kumar <shobhit.kumar@intel.com>

Added support for PSR entry counter and performance counters

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v2: Add psr enabled yes/no info
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 882a735..39513de 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1658,6 +1658,39 @@ static int i915_dpio_info(struct seq_file *m, void *data)
 	return 0;
 }
 
+static int i915_edp_psr_status(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int count;
+	u32 psrctl, psrstat, psrperf;
+
+	/* Bit 31 gives the PSR enabled */
+	psrctl = I915_READ(EDP_PSR_CTL);
+	seq_printf(m, "PSR Enabled: %s\n",
+		   yesno(psrctl & EDP_PSR_ENABLE));
+
+	/* Bits 19:16 gives the PSR entry count */
+	psrstat = I915_READ(EDP_PSR_STATUS_CTL);
+	count = ((psrstat >> 16) & 0xf);
+
+	/* Format the PSR Entry Count only for now.
+	 * TBD: Other status information
+	 */
+	seq_printf(m, "EDP_PSR_ENTRY_COUNT: %u\n", count);
+
+	/* Current PSR state */
+	count = ((psrstat >> 29) & 0x7);
+	seq_printf(m, "EDP_PSR_CURRENT_STATE: 0x%x\n", count);
+
+	/* Perfromance counter bit 23:0 */
+	psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & 0xffffff;
+	seq_printf(m, "EDP_PSR_PERF_COUNTER: %u\n", psrperf);
+
+	return 0;
+}
+
 static ssize_t
 i915_wedged_read(struct file *filp,
 		 char __user *ubuf,
@@ -2131,6 +2164,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_swizzle_info", i915_swizzle_info, 0},
 	{"i915_ppgtt_info", i915_ppgtt_info, 0},
 	{"i915_dpio", i915_dpio_info, 0},
+	{"i915_edp_psr_status", i915_edp_psr_status, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.7.11.7

[PATCH 8/8] drm/i915: Hook PSR functionality

[Rodrigo Vivi]

PSR must be enabled after transcoder and port are running.
And it is only available for HSW.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1464e47..7e1469a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3431,8 +3431,13 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	intel_crtc_update_cursor(crtc, true);
 
-	for_each_encoder_on_crtc(dev, crtc, encoder)
+	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		encoder->enable(encoder);
+		if (encoder->type == INTEL_OUTPUT_EDP) {
+			struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+			intel_edp_enable_psr(intel_dp);
+		}
+	}
 
 	/*
 	 * There seems to be a race in PCH platform hw (at least on some
@@ -3459,8 +3464,14 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	for_each_encoder_on_crtc(dev, crtc, encoder)
+	for_each_encoder_on_crtc(dev, crtc, encoder) {
+		if (encoder->type == INTEL_OUTPUT_EDP) {
+			struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+			intel_edp_disable_psr(intel_dp);
+		}
 		encoder->disable(encoder);
+	}
+
 
 	intel_crtc_wait_for_pending_flips(crtc);
 	drm_vblank_off(dev, pipe);
-- 
1.7.11.7

Re: [PATCH 8/8] drm/i915: Hook PSR functionality

[Daniel Vetter]

On Fri, Jan 11, 2013 at 05:57:57PM -0200, Rodrigo Vivi wrote:
> PSR must be enabled after transcoder and port are running.
> And it is only available for HSW.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1464e47..7e1469a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3431,8 +3431,13 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_crtc_update_cursor(crtc, true);
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder)
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
>  		encoder->enable(encoder);
> +		if (encoder->type == INTEL_OUTPUT_EDP) {
> +			struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +			intel_edp_enable_psr(intel_dp);
> +		}

This belongs into the dp encoder ->enable/disable callback. Really, I
_hate_ such encoder/output special-casing in our crtc code ;-) So we
should try to move away from such leaky abstraction, not add more ...
-Daniel

> +	}
>  
>  	/*
>  	 * There seems to be a race in PCH platform hw (at least on some
> @@ -3459,8 +3464,14 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	if (!intel_crtc->active)
>  		return;
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder)
> +	for_each_encoder_on_crtc(dev, crtc, encoder) {
> +		if (encoder->type == INTEL_OUTPUT_EDP) {
> +			struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +			intel_edp_disable_psr(intel_dp);
> +		}
>  		encoder->disable(encoder);
> +	}
> +
>  
>  	intel_crtc_wait_for_pending_flips(crtc);
>  	drm_vblank_off(dev, pipe);
> -- 
> 1.7.11.7
> 
> _______________________________________________
> 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

Re: [PATCH 8/8] drm/i915: Hook PSR functionality

[Rodrigo Vivi]

makes sense... I was in doubt about it and I had tested it already
inside  ddi_enable...
I'm just not sure about having a IS_HASWELL test before calling it...
I think this is the safest approach right?


On Fri, Jan 11, 2013 at 6:49 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Jan 11, 2013 at 05:57:57PM -0200, Rodrigo Vivi wrote:
>> PSR must be enabled after transcoder and port are running.
>> And it is only available for HSW.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 1464e47..7e1469a 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3431,8 +3431,13 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>>
>>       intel_crtc_update_cursor(crtc, true);
>>
>> -     for_each_encoder_on_crtc(dev, crtc, encoder)
>> +     for_each_encoder_on_crtc(dev, crtc, encoder) {
>>               encoder->enable(encoder);
>> +             if (encoder->type == INTEL_OUTPUT_EDP) {
>> +                     struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> +                     intel_edp_enable_psr(intel_dp);
>> +             }
>
> This belongs into the dp encoder ->enable/disable callback. Really, I
> _hate_ such encoder/output special-casing in our crtc code ;-) So we
> should try to move away from such leaky abstraction, not add more ...
> -Daniel
>
>> +     }
>>
>>       /*
>>        * There seems to be a race in PCH platform hw (at least on some
>> @@ -3459,8 +3464,14 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>>       if (!intel_crtc->active)
>>               return;
>>
>> -     for_each_encoder_on_crtc(dev, crtc, encoder)
>> +     for_each_encoder_on_crtc(dev, crtc, encoder) {
>> +             if (encoder->type == INTEL_OUTPUT_EDP) {
>> +                     struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> +                     intel_edp_disable_psr(intel_dp);
>> +             }
>>               encoder->disable(encoder);
>> +     }
>> +
>>
>>       intel_crtc_wait_for_pending_flips(crtc);
>>       drm_vblank_off(dev, pipe);
>> --
>> 1.7.11.7
>>
>> _______________________________________________
>> 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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH 8/8] drm/i915: Hook PSR functionality

[Daniel Vetter]

On Tue, Jan 15, 2013 at 2:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> makes sense... I was in doubt about it and I had tested it already
> inside  ddi_enable...
> I'm just not sure about having a IS_HASWELL test before calling it...
> I think this is the safest approach right?

Atm ddi is hsw-only, so overkill.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch