On Fri, Jun 02, 2017 at 11:55:32AM +0300, Jani Nikula wrote: > On Fri, 02 Jun 2017, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > > On Fri, 02 Jun 2017, Manasi Navare <manasi.d.navare@xxxxxxxxx> wrote: > >> Validate the compliance test link parameters when the compliance > >> test dpcd registers are read. Also validate them in compute_config > >> before using them since the max values might have been reduced > >> due to link training fallback. > >> > >> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/i915/intel_dp.c | 29 +++++++++++++---------------- > >> 1 file changed, 13 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > >> index 832786d..cda0f0a 100644 > >> --- a/drivers/gpu/drm/i915/intel_dp.c > >> +++ b/drivers/gpu/drm/i915/intel_dp.c > >> @@ -1678,12 +1678,15 @@ intel_dp_compute_config(struct intel_encoder *encoder, > >> if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) { > >> int index; > >> > >> - index = intel_dp_rate_index(intel_dp->common_rates, > >> - intel_dp->num_common_rates, > >> - intel_dp->compliance.test_link_rate); > >> - if (index >= 0) > >> + /* Validate the compliance test data */ > > Oh, and please do cite the reason for doing this again in the comment. I > first read the code, didn't get it, until I read the commit message. :) > > BR, > Jani. > Thanks for the review. Will add this reason in the comment. Will send a new revision with this change and checking index >=0. Regards Manasi > > >> + if (intel_dp_link_params_valid(intel_dp, intel_dp->compliance.test_link_rate, > >> + intel_dp->compliance.test_lane_count)) { > >> + index = intel_dp_rate_index(intel_dp->common_rates, > >> + intel_dp->num_common_rates, > >> + intel_dp->compliance.test_link_rate); > > > > I think you still need to check index >= 0. intel_dp_link_params_valid > > only checks the boundaries, not that the common rates actually contains > > the value. > > > >> min_clock = max_clock = index; > >> - min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count; > >> + min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count; > >> + } > >> } > >> DRM_DEBUG_KMS("DP link computation with max lane count %i " > >> "max bw %d pixel clock %iKHz\n", > >> @@ -3961,8 +3964,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) > >> static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) > >> { > >> int status = 0; > >> - int min_lane_count = 1; > >> - int link_rate_index, test_link_rate; > >> + int test_link_rate; > >> uint8_t test_lane_count, test_link_bw; > >> /* (DP CTS 1.2) > >> * 4.3.1.11 > >> @@ -3976,10 +3978,6 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) > >> return DP_TEST_NAK; > >> } > >> test_lane_count &= DP_MAX_LANE_COUNT_MASK; > >> - /* Validate the requested lane count */ > >> - if (test_lane_count < min_lane_count || > >> - test_lane_count > intel_dp->max_link_lane_count) > >> - return DP_TEST_NAK; > >> > >> status = drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_LINK_RATE, > >> &test_link_bw); > >> @@ -3987,12 +3985,11 @@ static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) > >> DRM_DEBUG_KMS("Link Rate read failed\n"); > >> return DP_TEST_NAK; > >> } > >> - /* Validate the requested link rate */ > >> test_link_rate = drm_dp_bw_code_to_link_rate(test_link_bw); > >> - link_rate_index = intel_dp_rate_index(intel_dp->common_rates, > >> - intel_dp->num_common_rates, > >> - test_link_rate); > >> - if (link_rate_index < 0) > >> + > >> + /* Validate the requested link rate and lane count */ > >> + if (!intel_dp_link_params_valid(intel_dp, test_link_rate, > >> + test_lane_count)) > >> return DP_TEST_NAK; > > > > The changes here LGTM. > > > >> > >> intel_dp->compliance.test_lane_count = test_lane_count; > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx