On Thu, Feb 02, 2017 at 10:42:48AM +0200, Jani Nikula wrote:
> On Thu, 02 Feb 2017, Manasi Navare <manasi.d.nav...@intel.com> wrote:
> > On Thu, Jan 26, 2017 at 09:44:26PM +0200, Jani Nikula wrote:
> >> Localize link_rate_index to the if block, and rename to just index to
> >> reduce indent.
> >>
> >> Cc: Manasi Navare <manasi.d.nav...@intel.com>
> >> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_dp.c | 13 +++++++------
> >> 1 file changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> index 7704d32286a3..429dc70c251a 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1639,7 +1639,6 @@ intel_dp_compute_config(struct intel_encoder
> >> *encoder,
> >> /* Conveniently, the link BW constants become indices with a shift...*/
> >> int min_clock = 0;
> >> int max_clock;
> >> - int link_rate_index;
> >> int bpp, mode_rate;
> >> int link_avail, link_clock;
> >> const int *common_rates = intel_dp->common_rates;
> >> @@ -1684,11 +1683,13 @@ intel_dp_compute_config(struct intel_encoder
> >> *encoder,
> >>
> >> /* Use values requested by Compliance Test Request */
> >> if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> >> - link_rate_index = intel_dp_find_rate(intel_dp->common_rates,
> >> - intel_dp->num_common_rates,
> >> -
> >> intel_dp->compliance.test_link_rate);
> >
> > Can we not pass just the common_rates as an argument to this function
> > since common_rates is already assigned intel_dp->common_rates.
>
> Do you mean just pass intel_dp to intel_dp_find_rate? If yes, I think
> this keeps intel_dp_find_rate generic, independent of intel_dp or
> anything else, and you can be sure it's stateless (same as
> intersect_rates).
>
> If you don't mean that, I don't know what you mean... please explain.
>
> BR,
> Jani.
>
>
I agree that it needs to be stateless and hence we dont pass intel_dp directly.
What I was saying was that at the beginning of the function we do initialize
common_rates = intel_dp->common_rates;
so why cant we pass just common_rates to this function instead of
intel_dp->common_rates?
Not sure if this optimization will make any big impact but just a thought.
Regards
Manasi
> >
> > Regards
> > Manasi
> >
> >
> >> - if (link_rate_index >= 0)
> >> - min_clock = max_clock = link_rate_index;
> >> + int index;
> >> +
> >> + index = intel_dp_find_rate(intel_dp->common_rates,
> >> + intel_dp->num_common_rates,
> >> + intel_dp->compliance.test_link_rate);
> >> + if (index >= 0)
> >> + min_clock = max_clock = index;
> >> min_lane_count = max_lane_count =
> >> intel_dp->compliance.test_lane_count;
> >> }
> >> DRM_DEBUG_KMS("DP link computation with max lane count %i "
> >> --
> >> 2.1.4
> >>
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
