| Message ID | 1438322573-13349-1-git-send-email-sivakumar.thulasimani@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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;
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
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
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
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
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
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
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;