| Message ID | 1439813711-2438-5-git-send-email-sivakumar.thulasimani@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 17 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote: > From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com> > > This patch fixes the bug that SKL SKUs before B0 might return > HBR2 as supported even though it is not supposed to be enabled > on such platforms. > > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 03523b3..963fdae 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1224,21 +1224,23 @@ static bool intel_dp_is_hbr2_supported(struct drm_device *dev) > static int > intel_dp_source_rates(struct drm_device *dev, const int **source_rates) > { > + int size = 0; No need to initialize. > if (IS_BROXTON(dev)) { > *source_rates = bxt_rates; > - return ARRAY_SIZE(bxt_rates); > + size = ARRAY_SIZE(bxt_rates); > } else if (IS_SKYLAKE(dev)) { > *source_rates = skl_rates; > - return ARRAY_SIZE(skl_rates); > + size = ARRAY_SIZE(skl_rates); > + } else { > + *source_rates = default_rates; > + size = ARRAY_SIZE(default_rates); > } > > - *source_rates = default_rates; > - > /* This depends on the fact that 5.4 is last value in the array */ > if (intel_dp_is_hbr2_supported(dev)) > - return (DP_LINK_BW_5_4 >> 3) + 1; > + return size; > else > - return (DP_LINK_BW_2_7 >> 3) + 1; > + return size - 1; /* This depends on the fact that 5.4 is last value in the array */ if (!intel_dp_source_supports_hbr2(dev)) size--; return size; > } > > static void > -- > 1.7.9.5 >
On Mon, Aug 17, 2015 at 05:45:11PM +0530, Sivakumar Thulasimani wrote: > From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com> > > This patch fixes the bug that SKL SKUs before B0 might return > HBR2 as supported even though it is not supposed to be enabled > on such platforms. > > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 03523b3..963fdae 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1224,21 +1224,23 @@ static bool intel_dp_is_hbr2_supported(struct drm_device *dev) > static int > intel_dp_source_rates(struct drm_device *dev, const int **source_rates) > { > + int size = 0; > if (IS_BROXTON(dev)) { > *source_rates = bxt_rates; > - return ARRAY_SIZE(bxt_rates); > + size = ARRAY_SIZE(bxt_rates); > } else if (IS_SKYLAKE(dev)) { > *source_rates = skl_rates; > - return ARRAY_SIZE(skl_rates); > + size = ARRAY_SIZE(skl_rates); > + } else { > + *source_rates = default_rates; > + size = ARRAY_SIZE(default_rates); > } > > - *source_rates = default_rates; > - > /* This depends on the fact that 5.4 is last value in the array */ > if (intel_dp_is_hbr2_supported(dev)) > - return (DP_LINK_BW_5_4 >> 3) + 1; > + return size; > else > - return (DP_LINK_BW_2_7 >> 3) + 1; > + return size - 1; Maybe we should use rate_to_index() here? Should be a bit more future proof for when we get HBR3. So, perhaps something like this? { ... *source_rates = bxt_rates; size_rates = ARRAY_SIZE(bxt_rates); ... if (intel_dp_is_hbr2_supported) max_rate = 540000; else max_rate = 270000; size = rate_to_index(max_rate, *source_rates) + 1; if (WARN_ON(size > size_rates)) size = size_rates; return size; } But that could be a followup patch. Otherwise the series looks good so: Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > } > > static void > -- > 1.7.9.5
On 8/17/2015 6:11 PM, Ville Syrjälä wrote: > On Mon, Aug 17, 2015 at 05:45:11PM +0530, Sivakumar Thulasimani wrote: >> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com> >> >> This patch fixes the bug that SKL SKUs before B0 might return >> HBR2 as supported even though it is not supposed to be enabled >> on such platforms. >> >> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 03523b3..963fdae 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -1224,21 +1224,23 @@ static bool intel_dp_is_hbr2_supported(struct drm_device *dev) >> static int >> intel_dp_source_rates(struct drm_device *dev, const int **source_rates) >> { >> + int size = 0; >> if (IS_BROXTON(dev)) { >> *source_rates = bxt_rates; >> - return ARRAY_SIZE(bxt_rates); >> + size = ARRAY_SIZE(bxt_rates); >> } else if (IS_SKYLAKE(dev)) { >> *source_rates = skl_rates; >> - return ARRAY_SIZE(skl_rates); >> + size = ARRAY_SIZE(skl_rates); >> + } else { >> + *source_rates = default_rates; >> + size = ARRAY_SIZE(default_rates); >> } >> >> - *source_rates = default_rates; >> - >> /* This depends on the fact that 5.4 is last value in the array */ >> if (intel_dp_is_hbr2_supported(dev)) >> - return (DP_LINK_BW_5_4 >> 3) + 1; >> + return size; >> else >> - return (DP_LINK_BW_2_7 >> 3) + 1; >> + return size - 1; > Maybe we should use rate_to_index() here? Should be a bit more > future proof for when we get HBR3. So, perhaps something like this? > > { > ... > *source_rates = bxt_rates; > size_rates = ARRAY_SIZE(bxt_rates); > ... > > if (intel_dp_is_hbr2_supported) > max_rate = 540000; > else > max_rate = 270000; > > size = rate_to_index(max_rate, *source_rates) + 1; > if (WARN_ON(size > size_rates)) > size = size_rates; > > return size; > } > > But that could be a followup patch. > > Otherwise the series looks good so: > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> thanks for the review :) >> } >> >> static void >> -- >> 1.7.9.5
On 8/17/2015 5:59 PM, Jani Nikula wrote: > On Mon, 17 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote: >> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com> >> >> This patch fixes the bug that SKL SKUs before B0 might return >> HBR2 as supported even though it is not supposed to be enabled >> on such platforms. >> >> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 03523b3..963fdae 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -1224,21 +1224,23 @@ static bool intel_dp_is_hbr2_supported(struct drm_device *dev) >> static int >> intel_dp_source_rates(struct drm_device *dev, const int **source_rates) >> { >> + int size = 0; > No need to initialize. > >> if (IS_BROXTON(dev)) { >> *source_rates = bxt_rates; >> - return ARRAY_SIZE(bxt_rates); >> + size = ARRAY_SIZE(bxt_rates); >> } else if (IS_SKYLAKE(dev)) { >> *source_rates = skl_rates; >> - return ARRAY_SIZE(skl_rates); >> + size = ARRAY_SIZE(skl_rates); >> + } else { >> + *source_rates = default_rates; >> + size = ARRAY_SIZE(default_rates); >> } >> >> - *source_rates = default_rates; >> - >> /* This depends on the fact that 5.4 is last value in the array */ >> if (intel_dp_is_hbr2_supported(dev)) >> - return (DP_LINK_BW_5_4 >> 3) + 1; >> + return size; >> else >> - return (DP_LINK_BW_2_7 >> 3) + 1; >> + return size - 1; > /* This depends on the fact that 5.4 is last value in the array */ > if (!intel_dp_source_supports_hbr2(dev)) > size--; > > return size; uploaded v2 of patches 3 & 4. thanks for the review :) >> } >> >> static void >> -- >> 1.7.9.5 >>
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7216
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK -1 283/283 282/283
SNB 315/315 315/315
IVB 336/336 336/336
BYT 283/283 283/283
HSW 378/378 378/378
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(1)
Note: You need to pay more attention to line start with '*'
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 03523b3..963fdae 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1224,21 +1224,23 @@ static bool intel_dp_is_hbr2_supported(struct drm_device *dev) static int intel_dp_source_rates(struct drm_device *dev, const int **source_rates) { + int size = 0; if (IS_BROXTON(dev)) { *source_rates = bxt_rates; - return ARRAY_SIZE(bxt_rates); + size = ARRAY_SIZE(bxt_rates); } else if (IS_SKYLAKE(dev)) { *source_rates = skl_rates; - return ARRAY_SIZE(skl_rates); + size = ARRAY_SIZE(skl_rates); + } else { + *source_rates = default_rates; + size = ARRAY_SIZE(default_rates); } - *source_rates = default_rates; - /* This depends on the fact that 5.4 is last value in the array */ if (intel_dp_is_hbr2_supported(dev)) - return (DP_LINK_BW_5_4 >> 3) + 1; + return size; else - return (DP_LINK_BW_2_7 >> 3) + 1; + return size - 1; } static void