[PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Sivakumar Thulasimani]

From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>

This reverts
commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Thu Mar 12 17:10:38 2015 +0200

CHV does not support intermediate frequencies so reverting the
patch that added it in the first place

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44f8a32..d9fb7a8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -95,9 +95,6 @@ static const int bxt_rates[] = { 162000, 216000, 243000, 270000,
 				  324000, 432000, 540000 };
 static const int skl_rates[] = { 162000, 216000, 270000,
 				  324000, 432000, 540000 };
-static const int chv_rates[] = { 162000, 202500, 210000, 216000,
-				 243000, 270000, 324000, 405000,
-				 420000, 432000, 540000 };
 static const int default_rates[] = { 162000, 270000, 540000 };
 
 /**
@@ -1185,9 +1182,6 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
 	} else if (IS_SKYLAKE(dev)) {
 		*source_rates = skl_rates;
 		return ARRAY_SIZE(skl_rates);
-	} else if (IS_CHERRYVIEW(dev)) {
-		*source_rates = chv_rates;
-		return ARRAY_SIZE(chv_rates);
 	}
 
 	*source_rates = default_rates;
-- 
1.7.9.5

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

[PATCH 2/2] drm/i915: remove HBR2 from chv supported list

[Sivakumar Thulasimani]

From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>

This patch removes 5.4Gbps from supported link rate for CHV since
it is not supported in it.

Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d9fb7a8..4e53433 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1186,8 +1186,9 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
 
 	*source_rates = default_rates;
 
-	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
-		/* WaDisableHBR2:skl */
+	/* WaDisableHBR2:skl */
+	if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
+		IS_CHERRYVIEW(dev))
 		return (DP_LINK_BW_2_7 >> 3) + 1;
 	else if (INTEL_INFO(dev)->gen >= 8 ||
 	    (IS_HASWELL(dev) && !IS_HSW_ULX(dev)))
-- 
1.7.9.5

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

Re: [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Sivakumar Thulasimani]

hi Ville,
   can you review these patches ?

regards,
Sivakumar

On Friday 31 July 2015 11:32 AM, Sivakumar Thulasimani wrote:
> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>
> This reverts
> commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Thu Mar 12 17:10:38 2015 +0200
>
> CHV does not support intermediate frequencies so reverting the
> patch that added it in the first place
>
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c |    6 ------
>   1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 44f8a32..d9fb7a8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -95,9 +95,6 @@ static const int bxt_rates[] = { 162000, 216000, 243000, 270000,
>   				  324000, 432000, 540000 };
>   static const int skl_rates[] = { 162000, 216000, 270000,
>   				  324000, 432000, 540000 };
> -static const int chv_rates[] = { 162000, 202500, 210000, 216000,
> -				 243000, 270000, 324000, 405000,
> -				 420000, 432000, 540000 };
>   static const int default_rates[] = { 162000, 270000, 540000 };
>   
>   /**
> @@ -1185,9 +1182,6 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>   	} else if (IS_SKYLAKE(dev)) {
>   		*source_rates = skl_rates;
>   		return ARRAY_SIZE(skl_rates);
> -	} else if (IS_CHERRYVIEW(dev)) {
> -		*source_rates = chv_rates;
> -		return ARRAY_SIZE(chv_rates);
>   	}
>   
>   	*source_rates = default_rates;

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

Re: [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Ville Syrjälä]

On Fri, Jul 31, 2015 at 11:32:52AM +0530, Sivakumar Thulasimani wrote:
> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
> 
> This reverts
> commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Thu Mar 12 17:10:38 2015 +0200
> 
> CHV does not support intermediate frequencies so reverting the
> patch that added it in the first place

My docs still say it does. Is there some undocumented problem with the
PLL or is this just a marketing decision?

> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 44f8a32..d9fb7a8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -95,9 +95,6 @@ static const int bxt_rates[] = { 162000, 216000, 243000, 270000,
>  				  324000, 432000, 540000 };
>  static const int skl_rates[] = { 162000, 216000, 270000,
>  				  324000, 432000, 540000 };
> -static const int chv_rates[] = { 162000, 202500, 210000, 216000,
> -				 243000, 270000, 324000, 405000,
> -				 420000, 432000, 540000 };
>  static const int default_rates[] = { 162000, 270000, 540000 };
>  
>  /**
> @@ -1185,9 +1182,6 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>  	} else if (IS_SKYLAKE(dev)) {
>  		*source_rates = skl_rates;
>  		return ARRAY_SIZE(skl_rates);
> -	} else if (IS_CHERRYVIEW(dev)) {
> -		*source_rates = chv_rates;
> -		return ARRAY_SIZE(chv_rates);
>  	}
>  
>  	*source_rates = default_rates;
> -- 
> 1.7.9.5

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Sivakumar Thulasimani]

On 8/12/2015 5:02 PM, Ville Syrjälä wrote:
> On Fri, Jul 31, 2015 at 11:32:52AM +0530, Sivakumar Thulasimani wrote:
>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>>
>> This reverts
>> commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
>> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Date:   Thu Mar 12 17:10:38 2015 +0200
>>
>> CHV does not support intermediate frequencies so reverting the
>> patch that added it in the first place
> My docs still say it does. Is there some undocumented problem with the
> PLL or is this just a marketing decision?
I don't think so, i too have just one document that shows the phy values 
for each of
the link rates but have not found any where else to say it is supported .
Also this is not tested by anyone including us from product team so i 
highly doubt
  it will even work.

regarding HBR2, it was supposed to land on a future stepping that never 
happened
so it is not supported at all.
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c |    6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 44f8a32..d9fb7a8 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -95,9 +95,6 @@ static const int bxt_rates[] = { 162000, 216000, 243000, 270000,
>>   				  324000, 432000, 540000 };
>>   static const int skl_rates[] = { 162000, 216000, 270000,
>>   				  324000, 432000, 540000 };
>> -static const int chv_rates[] = { 162000, 202500, 210000, 216000,
>> -				 243000, 270000, 324000, 405000,
>> -				 420000, 432000, 540000 };
>>   static const int default_rates[] = { 162000, 270000, 540000 };
>>   
>>   /**
>> @@ -1185,9 +1182,6 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>   	} else if (IS_SKYLAKE(dev)) {
>>   		*source_rates = skl_rates;
>>   		return ARRAY_SIZE(skl_rates);
>> -	} else if (IS_CHERRYVIEW(dev)) {
>> -		*source_rates = chv_rates;
>> -		return ARRAY_SIZE(chv_rates);
>>   	}
>>   
>>   	*source_rates = default_rates;
>> -- 
>> 1.7.9.5

-- 
regards,
Sivakumar

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

Re: [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Ville Syrjälä]

On Wed, Aug 12, 2015 at 05:31:55PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 8/12/2015 5:02 PM, Ville Syrjälä wrote:
> > On Fri, Jul 31, 2015 at 11:32:52AM +0530, Sivakumar Thulasimani wrote:
> >> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
> >>
> >> This reverts
> >> commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
> >> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Date:   Thu Mar 12 17:10:38 2015 +0200
> >>
> >> CHV does not support intermediate frequencies so reverting the
> >> patch that added it in the first place
> > My docs still say it does. Is there some undocumented problem with the
> > PLL or is this just a marketing decision?
> I don't think so, i too have just one document that shows the phy values 
> for each of
> the link rates but have not found any where else to say it is supported .

Display cluster HAS says they're supported. And since the spreadsheet
has them all in green I assume someone actually figured that they ought
to work.

> Also this is not tested by anyone including us from product team so i 
> highly doubt
>   it will even work.

Well if no one has tested them I guess we shouldn't try to use them. Not
a big loss in any case I suppose.

So based on that reasoning I can give an ack for this patch:
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> regarding HBR2, it was supposed to land on a future stepping that never 
> happened
> so it is not supported at all.

Hmm. Display Cluster HAS listed it as a stretch goal for early steppings. Apart
from that there isn't much else to go by. Excepth the training pattern 3
support, but now that I look again the new bit for that has disappeared
from the DP register in the spec. Or maybe I was looking at the k0 spec
when I added it. It's still in the k0 spec but as you say that's been
nuked.

In light of this, I think dropping HBR2 is reasonable. HBR is anyway
enough for 4k@30 with 4 lanes, so it's not like we really need HBR2.

And could you also cook up a patch to kill the training pattern 3
support for CHV? Should be mostly a revert of my original patch that
added it, but you probably need to adjust the use_tps3 condition a bit
too.


> >> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_dp.c |    6 ------
> >>   1 file changed, 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 44f8a32..d9fb7a8 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -95,9 +95,6 @@ static const int bxt_rates[] = { 162000, 216000, 243000, 270000,
> >>   				  324000, 432000, 540000 };
> >>   static const int skl_rates[] = { 162000, 216000, 270000,
> >>   				  324000, 432000, 540000 };
> >> -static const int chv_rates[] = { 162000, 202500, 210000, 216000,
> >> -				 243000, 270000, 324000, 405000,
> >> -				 420000, 432000, 540000 };
> >>   static const int default_rates[] = { 162000, 270000, 540000 };
> >>   
> >>   /**
> >> @@ -1185,9 +1182,6 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
> >>   	} else if (IS_SKYLAKE(dev)) {
> >>   		*source_rates = skl_rates;
> >>   		return ARRAY_SIZE(skl_rates);
> >> -	} else if (IS_CHERRYVIEW(dev)) {
> >> -		*source_rates = chv_rates;
> >> -		return ARRAY_SIZE(chv_rates);
> >>   	}
> >>   
> >>   	*source_rates = default_rates;
> >> -- 
> >> 1.7.9.5
> 
> -- 
> regards,
> Sivakumar

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: remove HBR2 from chv supported list

[Ville Syrjälä]

On Fri, Jul 31, 2015 at 11:32:53AM +0530, Sivakumar Thulasimani wrote:
> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
> 
> This patch removes 5.4Gbps from supported link rate for CHV since
> it is not supported in it.
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d9fb7a8..4e53433 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1186,8 +1186,9 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>  
>  	*source_rates = default_rates;
>  
> -	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
> -		/* WaDisableHBR2:skl */
> +	/* WaDisableHBR2:skl */
> +	if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) ||
> +		IS_CHERRYVIEW(dev))
>  		return (DP_LINK_BW_2_7 >> 3) + 1;
>  	else if (INTEL_INFO(dev)->gen >= 8 ||
>  	    (IS_HASWELL(dev) && !IS_HSW_ULX(dev)))

I would suggest reordering this a bit. Something likle this perhaps:

/* WaDisableHBR2:skl */
if (IS_SKYLAKE && ...)
	return (DP_LINK_BW_2_7 >> 3) + 1;

if ((IS_HASWELL && !IS_HSW_ULX) || IS_BROADWELL || gen >= 9)
	return (DP_LINK_BW_5_4 >> 3) + 1;
else
	return (DP_LINK_BW_2_7 >> 3) + 1;

IMO that makes it easier to see exactly which platforms support
HBR2, and it keeps the SKL w/a separated neatly for easy removal in the
near future (I'm assuming we want to kill the w/a based on the fact that
it's for an early stepping).

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Daniel Vetter]

On Wed, Aug 12, 2015 at 04:02:17PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 12, 2015 at 05:31:55PM +0530, Sivakumar Thulasimani wrote:
> > 
> > 
> > On 8/12/2015 5:02 PM, Ville Syrjälä wrote:
> > > On Fri, Jul 31, 2015 at 11:32:52AM +0530, Sivakumar Thulasimani wrote:
> > >> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
> > >>
> > >> This reverts
> > >> commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
> > >> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> Date:   Thu Mar 12 17:10:38 2015 +0200
> > >>
> > >> CHV does not support intermediate frequencies so reverting the
> > >> patch that added it in the first place
> > > My docs still say it does. Is there some undocumented problem with the
> > > PLL or is this just a marketing decision?
> > I don't think so, i too have just one document that shows the phy values 
> > for each of
> > the link rates but have not found any where else to say it is supported .
> 
> Display cluster HAS says they're supported. And since the spreadsheet
> has them all in green I assume someone actually figured that they ought
> to work.
> 
> > Also this is not tested by anyone including us from product team so i 
> > highly doubt
> >   it will even work.
> 
> Well if no one has tested them I guess we shouldn't try to use them. Not
> a big loss in any case I suppose.
> 
> So based on that reasoning I can give an ack for this patch:
> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > 
> > regarding HBR2, it was supposed to land on a future stepping that never 
> > happened
> > so it is not supported at all.
> 
> Hmm. Display Cluster HAS listed it as a stretch goal for early steppings. Apart
> from that there isn't much else to go by. Excepth the training pattern 3
> support, but now that I look again the new bit for that has disappeared
> from the DP register in the spec. Or maybe I was looking at the k0 spec
> when I added it. It's still in the k0 spec but as you say that's been
> nuked.
> 
> In light of this, I think dropping HBR2 is reasonable. HBR is anyway
> enough for 4k@30 with 4 lanes, so it's not like we really need HBR2.
> 
> And could you also cook up a patch to kill the training pattern 3
> support for CHV? Should be mostly a revert of my original patch that
> added it, but you probably need to adjust the use_tps3 condition a bit
> too.

Can we please grill the people responsible for this docs mess some more?

Patch itself is for Jani.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Jani Nikula]

On Wed, 12 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Aug 12, 2015 at 04:02:17PM +0300, Ville Syrjälä wrote:
>> On Wed, Aug 12, 2015 at 05:31:55PM +0530, Sivakumar Thulasimani wrote:
>> > 
>> > 
>> > On 8/12/2015 5:02 PM, Ville Syrjälä wrote:
>> > > On Fri, Jul 31, 2015 at 11:32:52AM +0530, Sivakumar Thulasimani wrote:
>> > >> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>> > >>
>> > >> This reverts
>> > >> commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
>> > >> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > >> Date:   Thu Mar 12 17:10:38 2015 +0200
>> > >>
>> > >> CHV does not support intermediate frequencies so reverting the
>> > >> patch that added it in the first place
>> > > My docs still say it does. Is there some undocumented problem with the
>> > > PLL or is this just a marketing decision?
>> > I don't think so, i too have just one document that shows the phy values 
>> > for each of
>> > the link rates but have not found any where else to say it is supported .
>> 
>> Display cluster HAS says they're supported. And since the spreadsheet
>> has them all in green I assume someone actually figured that they ought
>> to work.
>> 
>> > Also this is not tested by anyone including us from product team so i 
>> > highly doubt
>> >   it will even work.
>> 
>> Well if no one has tested them I guess we shouldn't try to use them. Not
>> a big loss in any case I suppose.
>> 
>> So based on that reasoning I can give an ack for this patch:
>> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> > 
>> > regarding HBR2, it was supposed to land on a future stepping that never 
>> > happened
>> > so it is not supported at all.
>> 
>> Hmm. Display Cluster HAS listed it as a stretch goal for early steppings. Apart
>> from that there isn't much else to go by. Excepth the training pattern 3
>> support, but now that I look again the new bit for that has disappeared
>> from the DP register in the spec. Or maybe I was looking at the k0 spec
>> when I added it. It's still in the k0 spec but as you say that's been
>> nuked.
>> 
>> In light of this, I think dropping HBR2 is reasonable. HBR is anyway
>> enough for 4k@30 with 4 lanes, so it's not like we really need HBR2.
>> 
>> And could you also cook up a patch to kill the training pattern 3
>> support for CHV? Should be mostly a revert of my original patch that
>> added it, but you probably need to adjust the use_tps3 condition a bit
>> too.
>
> Can we please grill the people responsible for this docs mess some more?
>
> Patch itself is for Jani.

There was some suggestions from Ville [1] to patch 1/2 that I haven't
seen a reply to.

BR,
Jani.

[1] http://mid.gmane.org/20150812131205.GW5176@intel.com



> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"

[Sivakumar Thulasimani]

On 8/14/2015 12:29 PM, Jani Nikula wrote:
> On Wed, 12 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Aug 12, 2015 at 04:02:17PM +0300, Ville Syrjälä wrote:
>>> On Wed, Aug 12, 2015 at 05:31:55PM +0530, Sivakumar Thulasimani wrote:
>>>>
>>>> On 8/12/2015 5:02 PM, Ville Syrjälä wrote:
>>>>> On Fri, Jul 31, 2015 at 11:32:52AM +0530, Sivakumar Thulasimani wrote:
>>>>>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>>>>>>
>>>>>> This reverts
>>>>>> commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
>>>>>> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>> Date:   Thu Mar 12 17:10:38 2015 +0200
>>>>>>
>>>>>> CHV does not support intermediate frequencies so reverting the
>>>>>> patch that added it in the first place
>>>>> My docs still say it does. Is there some undocumented problem with the
>>>>> PLL or is this just a marketing decision?
>>>> I don't think so, i too have just one document that shows the phy values
>>>> for each of
>>>> the link rates but have not found any where else to say it is supported .
>>> Display cluster HAS says they're supported. And since the spreadsheet
>>> has them all in green I assume someone actually figured that they ought
>>> to work.
>>>
>>>> Also this is not tested by anyone including us from product team so i
>>>> highly doubt
>>>>    it will even work.
>>> Well if no one has tested them I guess we shouldn't try to use them. Not
>>> a big loss in any case I suppose.
>>>
>>> So based on that reasoning I can give an ack for this patch:
>>> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>>> regarding HBR2, it was supposed to land on a future stepping that never
>>>> happened
>>>> so it is not supported at all.
>>> Hmm. Display Cluster HAS listed it as a stretch goal for early steppings. Apart
>>> from that there isn't much else to go by. Excepth the training pattern 3
>>> support, but now that I look again the new bit for that has disappeared
>>> from the DP register in the spec. Or maybe I was looking at the k0 spec
>>> when I added it. It's still in the k0 spec but as you say that's been
>>> nuked.
>>>
>>> In light of this, I think dropping HBR2 is reasonable. HBR is anyway
>>> enough for 4k@30 with 4 lanes, so it's not like we really need HBR2.
>>>
>>> And could you also cook up a patch to kill the training pattern 3
>>> support for CHV? Should be mostly a revert of my original patch that
>>> added it, but you probably need to adjust the use_tps3 condition a bit
>>> too.
>> Can we please grill the people responsible for this docs mess some more?
>>
>> Patch itself is for Jani.
> There was some suggestions from Ville [1] to patch 1/2 that I haven't
> seen a reply to.
>
> BR,
> Jani.
>
> [1] http://mid.gmane.org/20150812131205.GW5176@intel.com
>
>
yes, i can make that change. i was assuming that since Daniel's reply 
had message
"patch itself is for Jani" that you would pick it up :), will check once 
before waiting
next time.
>> -Daniel
>> -- 
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch

-- 
regards,
Sivakumar

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