On Fri, Mar 13, 2015 at 05:14:59PM +0530, sonika wrote:
>
> On Thursday 12 March 2015 08:40 PM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > intel_dp_compute_config() only really needs to know the rates supported
> > by both source and sink, so hide the raw source and sink arrays from it.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 37 +++++++++++++++++++++++--------------
> > 1 file changed, 23 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 61538f4..a88f932 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1192,9 +1192,9 @@ intel_dp_set_clock(struct intel_encoder *encoder,
> > }
> > }
> >
> > -static int intel_supported_rates(const int *source_rates, int source_len,
> > - const int *sink_rates, int sink_len,
> > - int *supported_rates)
> > +static int intersect_rates(const int *source_rates, int source_len,
> > + const int *sink_rates, int sink_len,
> > + int *supported_rates)
> > {
> > int i = 0, j = 0, k = 0;
> >
> > @@ -1213,6 +1213,21 @@ static int intel_supported_rates(const int *source_rates, int source_len,
> > return k;
> > }
> >
> > +static int intel_supported_rates(struct intel_dp *intel_dp,
> > + int *supported_rates)
> > +{
> > + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > + const int *source_rates, *sink_rates;
> > + int source_len, sink_len;
> > +
> > + sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> > + source_len = intel_dp_source_rates(dev, &source_rates);
> > +
> > + return intersect_rates(source_rates, source_len,
> > + sink_rates, sink_len,
> > + supported_rates);
> > +}
> > +
> Now when I am looking at it, the name "supported_rates", sounds
> confusing. Because first we are using it for sink_rates in
> intel_dp->supported_rates
> and then we use it to store the intersect_rates. Since sink_rates are
> termed supported_rates in the spec, can we remove the sink_rates altogether
> and have source_rates, sink_supported_rates and common_rates (or
> something else ?)
Hmm. Yeah. Maybe just rename to intel_dp->sink_rates? And common_rates
would seem like a decent name for the intersection.
>
> -Sonika
> > static int rate_to_index(int find, const int *rates)
> > {
> > int i = 0;
> > @@ -1243,17 +1258,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > int max_clock;
> > int bpp, mode_rate;
> > int link_avail, link_clock;
> > - const int *sink_rates;
> > - int supported_rates[8] = {0};
> > - const int *source_rates;
> > - int source_len, sink_len, supported_len;
> > -
> > - sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> > -
> > - source_len = intel_dp_source_rates(dev, &source_rates);
> > + int supported_rates[DP_MAX_SUPPORTED_RATES] = {};
> > + int supported_len;
> >
> > - supported_len = intel_supported_rates(source_rates, source_len,
> > - sink_rates, sink_len, supported_rates);
> > + supported_len = intel_supported_rates(intel_dp, supported_rates);
> >
> > /* No common link rates between source and sink */
> > WARN_ON(supported_len <= 0);
> > @@ -1352,7 +1360,8 @@ found:
> >
> > if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
> > intel_dp->rate_select =
> > - rate_to_index(supported_rates[clock], sink_rates);
> > + rate_to_index(supported_rates[clock],
> > + intel_dp->supported_rates);
> > intel_dp->link_bw = 0;
> > }
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
