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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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