diff mbox

[8/9] drm/i915/bxt: Enable BXT DSI dual link

Message ID 1486551058-22596-9-git-send-email-vidya.srinivas@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Srinivas, Vidya Feb. 8, 2017, 10:50 a.m. UTC
From: Uma Shankar <uma.shankar@intel.com>

Enable support for BXT DSI dual link mode.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
 drivers/gpu/drm/i915/intel_dsi.c | 27 ++++++++++++++++++---------
 2 files changed, 23 insertions(+), 9 deletions(-)

Comments

Paauwe, Bob J Feb. 15, 2017, 6:33 p.m. UTC | #1
On Wed, 8 Feb 2017 16:20:57 +0530
Vidya Srinivas <vidya.srinivas@intel.com> wrote:

> From: Uma Shankar <uma.shankar@intel.com>
> 
> Enable support for BXT DSI dual link mode.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>

Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
>  drivers/gpu/drm/i915/intel_dsi.c | 27 ++++++++++++++++++---------
>  2 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 07b1a2d..3b2925c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8425,6 +8425,7 @@ enum {
>  #define  LANE_CONFIGURATION_4LANE			(0 << 0)
>  #define  LANE_CONFIGURATION_DUAL_LINK_A			(1 << 0)
>  #define  LANE_CONFIGURATION_DUAL_LINK_B			(2 << 0)
> +#define  LANE_CONFIGURATION_DUAL_LINK_ENABLE		(1 << 0)
>  
>  #define _MIPIA_TEARING_CTRL			(VLV_DISPLAY_BASE + 0x61194)
>  #define _MIPIC_TEARING_CTRL			(VLV_DISPLAY_BASE + 0x61704)
> @@ -8758,6 +8759,10 @@ enum {
>  #define  READ_REQUEST_PRIORITY_HIGH			(3 << 3)
>  #define  RGB_FLIP_TO_BGR				(1 << 2)
>  
> +/* BXT has dual link Z inversion overlap field */
> +#define  BXT_PIXEL_OVERLAP_CNT_MASK			(0xf << 10)
> +#define  BXT_PIXEL_OVERLAP_CNT_SHIFT			10
> +
>  #define  BXT_PIPE_SELECT_SHIFT				7
>  #define  BXT_PIPE_SELECT_MASK				(7 << 7)
>  #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 12aeee1..60ca0b9 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -440,15 +440,24 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	enum port port;
> +	u32 temp;
>  
>  	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> -		u32 temp;
> -
> -		temp = I915_READ(VLV_CHICKEN_3);
> -		temp &= ~PIXEL_OVERLAP_CNT_MASK |
> +		if (IS_BROXTON(dev_priv)) {
> +			for_each_dsi_port(port, intel_dsi->ports) {
> +				temp = I915_READ(MIPI_CTRL(port));
> +				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
>  					intel_dsi->pixel_overlap <<
> -					PIXEL_OVERLAP_CNT_SHIFT;
> -		I915_WRITE(VLV_CHICKEN_3, temp);
> +					BXT_PIXEL_OVERLAP_CNT_SHIFT;
> +				I915_WRITE(MIPI_CTRL(port), temp);
> +			}
> +		} else {
> +			temp = I915_READ(VLV_CHICKEN_3);
> +			temp &= ~PIXEL_OVERLAP_CNT_MASK |
> +				intel_dsi->pixel_overlap <<
> +				PIXEL_OVERLAP_CNT_SHIFT;
> +			I915_WRITE(VLV_CHICKEN_3, temp);
> +		}
>  	}
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
> @@ -464,12 +473,12 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>  		if (intel_dsi->ports == (BIT(PORT_A) | BIT(PORT_C))) {
>  			temp |= (intel_dsi->dual_link - 1)
>  						<< DUAL_LINK_MODE_SHIFT;
> -			if (IS_BROXTON(dev_priv))
> -				temp |= LANE_CONFIGURATION_DUAL_LINK_A;
> -			else
> +			if (IS_VALLEYVIEW(dev_priv))
>  				temp |= intel_crtc->pipe ?
>  					LANE_CONFIGURATION_DUAL_LINK_B :
>  					LANE_CONFIGURATION_DUAL_LINK_A;
> +			else if (IS_BROXTON(dev_priv))
> +				temp |= LANE_CONFIGURATION_DUAL_LINK_ENABLE;
>  		}
>  		/* assert ip_tg_enable signal */
>  		I915_WRITE(port_ctrl, temp | DPI_ENABLE);
Jani Nikula Feb. 16, 2017, 3:26 p.m. UTC | #2
On Wed, 15 Feb 2017, Bob Paauwe <bob.j.paauwe@intel.com> wrote:
> On Wed, 8 Feb 2017 16:20:57 +0530
> Vidya Srinivas <vidya.srinivas@intel.com> wrote:
>
>> From: Uma Shankar <uma.shankar@intel.com>
>> 
>> Enable support for BXT DSI dual link mode.
>> 
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>
> Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>

This doesn't apply because I've pushed

commit 6043801f937ada9c9ed9dfa3c6ce542a79643401
Author: Deepak M <m.deepak@intel.com>
Date:   Tue Feb 14 18:46:16 2017 +0530

    drm/i915: Set the Z inversion overlap field

Please update the patch, and in general, please send a new series of
patches that do *not* depend on the drm_panel changes. We can get all of
those merged first. We should try to make progress.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
>>  drivers/gpu/drm/i915/intel_dsi.c | 27 ++++++++++++++++++---------
>>  2 files changed, 23 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 07b1a2d..3b2925c 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -8425,6 +8425,7 @@ enum {
>>  #define  LANE_CONFIGURATION_4LANE			(0 << 0)
>>  #define  LANE_CONFIGURATION_DUAL_LINK_A			(1 << 0)
>>  #define  LANE_CONFIGURATION_DUAL_LINK_B			(2 << 0)
>> +#define  LANE_CONFIGURATION_DUAL_LINK_ENABLE		(1 << 0)
>>  
>>  #define _MIPIA_TEARING_CTRL			(VLV_DISPLAY_BASE + 0x61194)
>>  #define _MIPIC_TEARING_CTRL			(VLV_DISPLAY_BASE + 0x61704)
>> @@ -8758,6 +8759,10 @@ enum {
>>  #define  READ_REQUEST_PRIORITY_HIGH			(3 << 3)
>>  #define  RGB_FLIP_TO_BGR				(1 << 2)
>>  
>> +/* BXT has dual link Z inversion overlap field */
>> +#define  BXT_PIXEL_OVERLAP_CNT_MASK			(0xf << 10)
>> +#define  BXT_PIXEL_OVERLAP_CNT_SHIFT			10
>> +
>>  #define  BXT_PIPE_SELECT_SHIFT				7
>>  #define  BXT_PIPE_SELECT_MASK				(7 << 7)
>>  #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 12aeee1..60ca0b9 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -440,15 +440,24 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>  	enum port port;
>> +	u32 temp;
>>  
>>  	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
>> -		u32 temp;
>> -
>> -		temp = I915_READ(VLV_CHICKEN_3);
>> -		temp &= ~PIXEL_OVERLAP_CNT_MASK |
>> +		if (IS_BROXTON(dev_priv)) {
>> +			for_each_dsi_port(port, intel_dsi->ports) {
>> +				temp = I915_READ(MIPI_CTRL(port));
>> +				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
>>  					intel_dsi->pixel_overlap <<
>> -					PIXEL_OVERLAP_CNT_SHIFT;
>> -		I915_WRITE(VLV_CHICKEN_3, temp);
>> +					BXT_PIXEL_OVERLAP_CNT_SHIFT;
>> +				I915_WRITE(MIPI_CTRL(port), temp);
>> +			}
>> +		} else {
>> +			temp = I915_READ(VLV_CHICKEN_3);
>> +			temp &= ~PIXEL_OVERLAP_CNT_MASK |
>> +				intel_dsi->pixel_overlap <<
>> +				PIXEL_OVERLAP_CNT_SHIFT;
>> +			I915_WRITE(VLV_CHICKEN_3, temp);
>> +		}
>>  	}
>>  
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>> @@ -464,12 +473,12 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>>  		if (intel_dsi->ports == (BIT(PORT_A) | BIT(PORT_C))) {
>>  			temp |= (intel_dsi->dual_link - 1)
>>  						<< DUAL_LINK_MODE_SHIFT;
>> -			if (IS_BROXTON(dev_priv))
>> -				temp |= LANE_CONFIGURATION_DUAL_LINK_A;
>> -			else
>> +			if (IS_VALLEYVIEW(dev_priv))
>>  				temp |= intel_crtc->pipe ?
>>  					LANE_CONFIGURATION_DUAL_LINK_B :
>>  					LANE_CONFIGURATION_DUAL_LINK_A;
>> +			else if (IS_BROXTON(dev_priv))
>> +				temp |= LANE_CONFIGURATION_DUAL_LINK_ENABLE;
>>  		}
>>  		/* assert ip_tg_enable signal */
>>  		I915_WRITE(port_ctrl, temp | DPI_ENABLE);
Srinivas, Vidya Feb. 20, 2017, 9:49 a.m. UTC | #3
> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, February 16, 2017 8:56 PM
> To: Paauwe, Bob J <bob.j.paauwe@intel.com>; Srinivas, Vidya
> <vidya.srinivas@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 8/9] drm/i915/bxt: Enable BXT DSI dual link
> 
> On Wed, 15 Feb 2017, Bob Paauwe <bob.j.paauwe@intel.com> wrote:
> > On Wed, 8 Feb 2017 16:20:57 +0530
> > Vidya Srinivas <vidya.srinivas@intel.com> wrote:
> >
> >> From: Uma Shankar <uma.shankar@intel.com>
> >>
> >> Enable support for BXT DSI dual link mode.
> >>
> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >
> > Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> 
> This doesn't apply because I've pushed
> 
> commit 6043801f937ada9c9ed9dfa3c6ce542a79643401
> Author: Deepak M <m.deepak@intel.com>
> Date:   Tue Feb 14 18:46:16 2017 +0530
> 
>     drm/i915: Set the Z inversion overlap field
> 
> Please update the patch, and in general, please send a new series of patches
> that do *not* depend on the drm_panel changes. We can get all of those
> merged first. We should try to make progress.
> 
> BR,
> Jani.

Thanks Jani. I will rebase and re-submit and also  to remove drm_panel
interface dependency, I am planning to create panel sequence callbacks
in intel_dsi structure itself. Is this approach okay?

Regards
Vidya

> 
> 
> >
> >> ---
> >>  drivers/gpu/drm/i915/i915_reg.h  |  5 +++++
> >> drivers/gpu/drm/i915/intel_dsi.c | 27 ++++++++++++++++++---------
> >>  2 files changed, 23 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> b/drivers/gpu/drm/i915/i915_reg.h index 07b1a2d..3b2925c 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -8425,6 +8425,7 @@ enum {
> >>  #define  LANE_CONFIGURATION_4LANE			(0 << 0)
> >>  #define  LANE_CONFIGURATION_DUAL_LINK_A			(1 <<
> 0)
> >>  #define  LANE_CONFIGURATION_DUAL_LINK_B			(2 <<
> 0)
> >> +#define  LANE_CONFIGURATION_DUAL_LINK_ENABLE		(1 <<
> 0)
> >>
> >>  #define _MIPIA_TEARING_CTRL			(VLV_DISPLAY_BASE
> + 0x61194)
> >>  #define _MIPIC_TEARING_CTRL			(VLV_DISPLAY_BASE
> + 0x61704)
> >> @@ -8758,6 +8759,10 @@ enum {
> >>  #define  READ_REQUEST_PRIORITY_HIGH			(3 << 3)
> >>  #define  RGB_FLIP_TO_BGR				(1 << 2)
> >>
> >> +/* BXT has dual link Z inversion overlap field */
> >> +#define  BXT_PIXEL_OVERLAP_CNT_MASK			(0xf << 10)
> >> +#define  BXT_PIXEL_OVERLAP_CNT_SHIFT			10
> >> +
> >>  #define  BXT_PIPE_SELECT_SHIFT				7
> >>  #define  BXT_PIPE_SELECT_MASK				(7 << 7)
> >>  #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
> >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >> b/drivers/gpu/drm/i915/intel_dsi.c
> >> index 12aeee1..60ca0b9 100644
> >> --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> @@ -440,15 +440,24 @@ static void intel_dsi_port_enable(struct
> intel_encoder *encoder)
> >>  	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> >>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> >>  	enum port port;
> >> +	u32 temp;
> >>
> >>  	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
> >> -		u32 temp;
> >> -
> >> -		temp = I915_READ(VLV_CHICKEN_3);
> >> -		temp &= ~PIXEL_OVERLAP_CNT_MASK |
> >> +		if (IS_BROXTON(dev_priv)) {
> >> +			for_each_dsi_port(port, intel_dsi->ports) {
> >> +				temp = I915_READ(MIPI_CTRL(port));
> >> +				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK
> |
> >>  					intel_dsi->pixel_overlap <<
> >> -					PIXEL_OVERLAP_CNT_SHIFT;
> >> -		I915_WRITE(VLV_CHICKEN_3, temp);
> >> +					BXT_PIXEL_OVERLAP_CNT_SHIFT;
> >> +				I915_WRITE(MIPI_CTRL(port), temp);
> >> +			}
> >> +		} else {
> >> +			temp = I915_READ(VLV_CHICKEN_3);
> >> +			temp &= ~PIXEL_OVERLAP_CNT_MASK |
> >> +				intel_dsi->pixel_overlap <<
> >> +				PIXEL_OVERLAP_CNT_SHIFT;
> >> +			I915_WRITE(VLV_CHICKEN_3, temp);
> >> +		}
> >>  	}
> >>
> >>  	for_each_dsi_port(port, intel_dsi->ports) { @@ -464,12 +473,12
> @@
> >> static void intel_dsi_port_enable(struct intel_encoder *encoder)
> >>  		if (intel_dsi->ports == (BIT(PORT_A) | BIT(PORT_C))) {
> >>  			temp |= (intel_dsi->dual_link - 1)
> >>  						<<
> DUAL_LINK_MODE_SHIFT;
> >> -			if (IS_BROXTON(dev_priv))
> >> -				temp |=
> LANE_CONFIGURATION_DUAL_LINK_A;
> >> -			else
> >> +			if (IS_VALLEYVIEW(dev_priv))
> >>  				temp |= intel_crtc->pipe ?
> >>
> 	LANE_CONFIGURATION_DUAL_LINK_B :
> >>
> 	LANE_CONFIGURATION_DUAL_LINK_A;
> >> +			else if (IS_BROXTON(dev_priv))
> >> +				temp |=
> LANE_CONFIGURATION_DUAL_LINK_ENABLE;
> >>  		}
> >>  		/* assert ip_tg_enable signal */
> >>  		I915_WRITE(port_ctrl, temp | DPI_ENABLE);
> 
> --
> Jani Nikula, Intel Open Source Technology Center
Jani Nikula Feb. 20, 2017, 11 a.m. UTC | #4
On Mon, 20 Feb 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote:
> Thanks Jani. I will rebase and re-submit and also  to remove drm_panel
> interface dependency, I am planning to create panel sequence callbacks
> in intel_dsi structure itself. Is this approach okay?

I think that's unnecessary overhead. I've come to think we should just
do what Hans suggested in his patch [1]. It's easiest, and we don't
really benefit anything from the drm_panel interface or function pointer
chasing.

Hans, do you think you'll have the time or motivation to refresh your
series, or should we just let Vidya do this?

As an added difficulty, there's Madhav doing the Geminilake enabling at
the same time, and these two/three series [2][3][4] are bound to
conflict to some extent.

BR,
Jani.


[1] http://patchwork.freedesktop.org/patch/msgid/20161201202925.12220-10-hdegoede@redhat.com
[2] http://mid.mail-archive.com/20161201202925.12220-1-hdegoede@redhat.com
[3] http://mid.mail-archive.com/1486551058-22596-1-git-send-email-vidya.srinivas@intel.com
[4] http://mid.mail-archive.com/1487335415-14766-1-git-send-email-madhav.chauhan@intel.com
Hans de Goede Feb. 20, 2017, 11:40 a.m. UTC | #5
Hi,

On 20-02-17 12:00, Jani Nikula wrote:
> On Mon, 20 Feb 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote:
>> Thanks Jani. I will rebase and re-submit and also  to remove drm_panel
>> interface dependency, I am planning to create panel sequence callbacks
>> in intel_dsi structure itself. Is this approach okay?
>
> I think that's unnecessary overhead. I've come to think we should just
> do what Hans suggested in his patch [1]. It's easiest, and we don't
> really benefit anything from the drm_panel interface or function pointer
> chasing.
>
> Hans, do you think you'll have the time or motivation to refresh your
> series, or should we just let Vidya do this?
>
> As an added difficulty, there's Madhav doing the Geminilake enabling at
> the same time, and these two/three series [2][3][4] are bound to
> conflict to some extent.

I've my series rebased in a personal repo. As I was planning on resubmitting
it at some point in the future. I can send out a new version right now if
you want ...

Regards,

Hans
Jani Nikula Feb. 20, 2017, 11:55 a.m. UTC | #6
On Mon, 20 Feb 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 20-02-17 12:00, Jani Nikula wrote:
>> On Mon, 20 Feb 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote:
>>> Thanks Jani. I will rebase and re-submit and also  to remove drm_panel
>>> interface dependency, I am planning to create panel sequence callbacks
>>> in intel_dsi structure itself. Is this approach okay?
>>
>> I think that's unnecessary overhead. I've come to think we should just
>> do what Hans suggested in his patch [1]. It's easiest, and we don't
>> really benefit anything from the drm_panel interface or function pointer
>> chasing.
>>
>> Hans, do you think you'll have the time or motivation to refresh your
>> series, or should we just let Vidya do this?
>>
>> As an added difficulty, there's Madhav doing the Geminilake enabling at
>> the same time, and these two/three series [2][3][4] are bound to
>> conflict to some extent.
>
> I've my series rebased in a personal repo. As I was planning on resubmitting
> it at some point in the future. I can send out a new version right now if
> you want ...

Please do!

BR,
Jani.


>
> Regards,
>
> Hans
Jani Nikula Feb. 20, 2017, 7:19 p.m. UTC | #7
On Mon, 20 Feb 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 20-02-17 12:00, Jani Nikula wrote:
>> On Mon, 20 Feb 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com> wrote:
>>> Thanks Jani. I will rebase and re-submit and also  to remove drm_panel
>>> interface dependency, I am planning to create panel sequence callbacks
>>> in intel_dsi structure itself. Is this approach okay?
>>
>> I think that's unnecessary overhead. I've come to think we should just
>> do what Hans suggested in his patch [1]. It's easiest, and we don't
>> really benefit anything from the drm_panel interface or function pointer
>> chasing.
>>
>> Hans, do you think you'll have the time or motivation to refresh your
>> series, or should we just let Vidya do this?
>>
>> As an added difficulty, there's Madhav doing the Geminilake enabling at
>> the same time, and these two/three series [2][3][4] are bound to
>> conflict to some extent.
>
> I've my series rebased in a personal repo. As I was planning on resubmitting
> it at some point in the future. I can send out a new version right now if
> you want ...

Vidya, see [1] for Hans' series. I'm inclined to pick (most of) that,
and continue from there. Thoughts?

BR,
Jani.


[1] http://mid.mail-archive.com/20170220140845.1714-1-hdegoede@redhat.com
Srinivas, Vidya Feb. 21, 2017, 6:21 a.m. UTC | #8
> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, February 21, 2017 12:49 AM
> To: Hans de Goede <hdegoede@redhat.com>; Srinivas, Vidya
> <vidya.srinivas@intel.com>; Paauwe, Bob J <bob.j.paauwe@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Chauhan, Madhav
> <madhav.chauhan@intel.com>
> Subject: Re: [Intel-gfx] [PATCH 8/9] drm/i915/bxt: Enable BXT DSI dual link
> 
> On Mon, 20 Feb 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi,
> >
> > On 20-02-17 12:00, Jani Nikula wrote:
> >> On Mon, 20 Feb 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com>
> wrote:
> >>> Thanks Jani. I will rebase and re-submit and also  to remove
> >>> drm_panel interface dependency, I am planning to create panel
> >>> sequence callbacks in intel_dsi structure itself. Is this approach okay?
> >>
> >> I think that's unnecessary overhead. I've come to think we should
> >> just do what Hans suggested in his patch [1]. It's easiest, and we
> >> don't really benefit anything from the drm_panel interface or
> >> function pointer chasing.
> >>
> >> Hans, do you think you'll have the time or motivation to refresh your
> >> series, or should we just let Vidya do this?
> >>
> >> As an added difficulty, there's Madhav doing the Geminilake enabling
> >> at the same time, and these two/three series [2][3][4] are bound to
> >> conflict to some extent.
> >
> > I've my series rebased in a personal repo. As I was planning on
> > resubmitting it at some point in the future. I can send out a new
> > version right now if you want ...
> 
> Vidya, see [1] for Hans' series. I'm inclined to pick (most of) that, and
> continue from there. Thoughts?
> 
> BR,
> Jani.

We went through Hans' series. 
http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg113101.html
We feel its okay to call the panel sequences directly instead of introducing
new call backs. We can use this approach. But, Hans' series has other changes
as well. For now, can we have only the generic sequence changes merged and
we will rebase our changes on top of that.

Regards
Vidya
> 
> 
> [1] http://mid.mail-archive.com/20170220140845.1714-1-
> hdegoede@redhat.com
> 
> 
> --
> Jani Nikula, Intel Open Source Technology Center
Hans de Goede Feb. 21, 2017, 7:30 a.m. UTC | #9
Hi,

On 21-02-17 07:21, Srinivas, Vidya wrote:
>
>
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Tuesday, February 21, 2017 12:49 AM
>> To: Hans de Goede <hdegoede@redhat.com>; Srinivas, Vidya
>> <vidya.srinivas@intel.com>; Paauwe, Bob J <bob.j.paauwe@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; Chauhan, Madhav
>> <madhav.chauhan@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH 8/9] drm/i915/bxt: Enable BXT DSI dual link
>>
>> On Mon, 20 Feb 2017, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 20-02-17 12:00, Jani Nikula wrote:
>>>> On Mon, 20 Feb 2017, "Srinivas, Vidya" <vidya.srinivas@intel.com>
>> wrote:
>>>>> Thanks Jani. I will rebase and re-submit and also  to remove
>>>>> drm_panel interface dependency, I am planning to create panel
>>>>> sequence callbacks in intel_dsi structure itself. Is this approach okay?
>>>>
>>>> I think that's unnecessary overhead. I've come to think we should
>>>> just do what Hans suggested in his patch [1]. It's easiest, and we
>>>> don't really benefit anything from the drm_panel interface or
>>>> function pointer chasing.
>>>>
>>>> Hans, do you think you'll have the time or motivation to refresh your
>>>> series, or should we just let Vidya do this?
>>>>
>>>> As an added difficulty, there's Madhav doing the Geminilake enabling
>>>> at the same time, and these two/three series [2][3][4] are bound to
>>>> conflict to some extent.
>>>
>>> I've my series rebased in a personal repo. As I was planning on
>>> resubmitting it at some point in the future. I can send out a new
>>> version right now if you want ...
>>
>> Vidya, see [1] for Hans' series. I'm inclined to pick (most of) that, and
>> continue from there. Thoughts?
>>
>> BR,
>> Jani.
>
> We went through Hans' series.
> http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg113101.html
> We feel its okay to call the panel sequences directly instead of introducing
> new call backs. We can use this approach. But, Hans' series has other changes
> as well.

Right, changes to actually make the driver follow the spec, instead of calling
a number of sequences at the wrong time, these seem like worthwhile changes
to have and it would be good to land them sooner rather then later so that
most bxt qa will be done with the sequences called at the proper times.

Regards,

Hans


  For now, can we have only the generic sequence changes merged and
> we will rebase our changes on top of that.
>
> Regards
> Vidya
>>
>>
>> [1] http://mid.mail-archive.com/20170220140845.1714-1-
>> hdegoede@redhat.com
>>
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 07b1a2d..3b2925c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8425,6 +8425,7 @@  enum {
 #define  LANE_CONFIGURATION_4LANE			(0 << 0)
 #define  LANE_CONFIGURATION_DUAL_LINK_A			(1 << 0)
 #define  LANE_CONFIGURATION_DUAL_LINK_B			(2 << 0)
+#define  LANE_CONFIGURATION_DUAL_LINK_ENABLE		(1 << 0)
 
 #define _MIPIA_TEARING_CTRL			(VLV_DISPLAY_BASE + 0x61194)
 #define _MIPIC_TEARING_CTRL			(VLV_DISPLAY_BASE + 0x61704)
@@ -8758,6 +8759,10 @@  enum {
 #define  READ_REQUEST_PRIORITY_HIGH			(3 << 3)
 #define  RGB_FLIP_TO_BGR				(1 << 2)
 
+/* BXT has dual link Z inversion overlap field */
+#define  BXT_PIXEL_OVERLAP_CNT_MASK			(0xf << 10)
+#define  BXT_PIXEL_OVERLAP_CNT_SHIFT			10
+
 #define  BXT_PIPE_SELECT_SHIFT				7
 #define  BXT_PIPE_SELECT_MASK				(7 << 7)
 #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 12aeee1..60ca0b9 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -440,15 +440,24 @@  static void intel_dsi_port_enable(struct intel_encoder *encoder)
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	enum port port;
+	u32 temp;
 
 	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
-		u32 temp;
-
-		temp = I915_READ(VLV_CHICKEN_3);
-		temp &= ~PIXEL_OVERLAP_CNT_MASK |
+		if (IS_BROXTON(dev_priv)) {
+			for_each_dsi_port(port, intel_dsi->ports) {
+				temp = I915_READ(MIPI_CTRL(port));
+				temp &= ~BXT_PIXEL_OVERLAP_CNT_MASK |
 					intel_dsi->pixel_overlap <<
-					PIXEL_OVERLAP_CNT_SHIFT;
-		I915_WRITE(VLV_CHICKEN_3, temp);
+					BXT_PIXEL_OVERLAP_CNT_SHIFT;
+				I915_WRITE(MIPI_CTRL(port), temp);
+			}
+		} else {
+			temp = I915_READ(VLV_CHICKEN_3);
+			temp &= ~PIXEL_OVERLAP_CNT_MASK |
+				intel_dsi->pixel_overlap <<
+				PIXEL_OVERLAP_CNT_SHIFT;
+			I915_WRITE(VLV_CHICKEN_3, temp);
+		}
 	}
 
 	for_each_dsi_port(port, intel_dsi->ports) {
@@ -464,12 +473,12 @@  static void intel_dsi_port_enable(struct intel_encoder *encoder)
 		if (intel_dsi->ports == (BIT(PORT_A) | BIT(PORT_C))) {
 			temp |= (intel_dsi->dual_link - 1)
 						<< DUAL_LINK_MODE_SHIFT;
-			if (IS_BROXTON(dev_priv))
-				temp |= LANE_CONFIGURATION_DUAL_LINK_A;
-			else
+			if (IS_VALLEYVIEW(dev_priv))
 				temp |= intel_crtc->pipe ?
 					LANE_CONFIGURATION_DUAL_LINK_B :
 					LANE_CONFIGURATION_DUAL_LINK_A;
+			else if (IS_BROXTON(dev_priv))
+				temp |= LANE_CONFIGURATION_DUAL_LINK_ENABLE;
 		}
 		/* assert ip_tg_enable signal */
 		I915_WRITE(port_ctrl, temp | DPI_ENABLE);