* [PATCH 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated @ 2017-06-02 0:51 Manasi Navare 2017-06-02 0:51 ` [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters Manasi Navare ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Manasi Navare @ 2017-06-02 0:51 UTC (permalink / raw) To: intel-gfx This function now takes the link rate and lane ocunt to be validated as an argument so that this can be used for validating even the compliance test link parameters. Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.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 58dca87..832786d 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -322,19 +322,20 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp, return 0; } -static bool intel_dp_link_params_valid(struct intel_dp *intel_dp) +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate, + uint8_t lane_count) { /* * FIXME: we need to synchronize the current link parameters with * hardware readout. Currently fast link training doesn't work on * boot-up. */ - if (intel_dp->link_rate == 0 || - intel_dp->link_rate > intel_dp->max_link_rate) + if (link_rate == 0 || + link_rate > intel_dp->max_link_rate) return false; - if (intel_dp->lane_count == 0 || - intel_dp->lane_count > intel_dp_max_lane_count(intel_dp)) + if (lane_count == 0 || + lane_count > intel_dp_max_lane_count(intel_dp)) return false; return true; @@ -4260,7 +4261,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) * Validate the cached values of intel_dp->link_rate and * intel_dp->lane_count before attempting to retrain. */ - if (!intel_dp_link_params_valid(intel_dp)) + if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate, + intel_dp->lane_count)) return; /* Retrain if Channel EQ or CR not ok */ -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters 2017-06-02 0:51 [PATCH 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated Manasi Navare @ 2017-06-02 0:51 ` Manasi Navare 2017-06-02 8:27 ` Jani Nikula 2017-06-02 10:34 ` Ville Syrjälä 2017-06-02 1:04 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated Patchwork 2017-06-02 8:22 ` [PATCH 1/2] " Jani Nikula 2 siblings, 2 replies; 10+ messages in thread From: Manasi Navare @ 2017-06-02 0:51 UTC (permalink / raw) To: intel-gfx 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@intel.com> --- 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 */ + 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); 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; intel_dp->compliance.test_lane_count = test_lane_count; -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters 2017-06-02 0:51 ` [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters Manasi Navare @ 2017-06-02 8:27 ` Jani Nikula 2017-06-02 8:55 ` Jani Nikula 2017-06-05 18:27 ` Manasi Navare 2017-06-02 10:34 ` Ville Syrjälä 1 sibling, 2 replies; 10+ messages in thread From: Jani Nikula @ 2017-06-02 8:27 UTC (permalink / raw) To: Manasi Navare, intel-gfx On Fri, 02 Jun 2017, Manasi Navare <manasi.d.navare@intel.com> 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@intel.com> > --- > 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 */ > + 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters 2017-06-02 8:27 ` Jani Nikula @ 2017-06-02 8:55 ` Jani Nikula 2017-06-08 18:36 ` Manasi Navare 2017-06-05 18:27 ` Manasi Navare 1 sibling, 1 reply; 10+ messages in thread From: Jani Nikula @ 2017-06-02 8:55 UTC (permalink / raw) To: Manasi Navare, intel-gfx On Fri, 02 Jun 2017, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 02 Jun 2017, Manasi Navare <manasi.d.navare@intel.com> 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@intel.com> >> --- >> 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. >> + 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters 2017-06-02 8:55 ` Jani Nikula @ 2017-06-08 18:36 ` Manasi Navare 0 siblings, 0 replies; 10+ messages in thread From: Manasi Navare @ 2017-06-08 18:36 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Fri, Jun 02, 2017 at 11:55:32AM +0300, Jani Nikula wrote: > On Fri, 02 Jun 2017, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 02 Jun 2017, Manasi Navare <manasi.d.navare@intel.com> 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@intel.com> > >> --- > >> 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters 2017-06-02 8:27 ` Jani Nikula 2017-06-02 8:55 ` Jani Nikula @ 2017-06-05 18:27 ` Manasi Navare 1 sibling, 0 replies; 10+ messages in thread From: Manasi Navare @ 2017-06-05 18:27 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Fri, Jun 02, 2017 at 11:27:40AM +0300, Jani Nikula wrote: > On Fri, 02 Jun 2017, Manasi Navare <manasi.d.navare@intel.com> 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@intel.com> > > --- > > 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 */ > > + 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. > I thought that it would be implied that if it falls within the bounds then it would be part of the common_rates array. But yes no harm in checking the index >=0 here again. I will add this check. Manasi > > 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters 2017-06-02 0:51 ` [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters Manasi Navare 2017-06-02 8:27 ` Jani Nikula @ 2017-06-02 10:34 ` Ville Syrjälä 2017-06-05 18:39 ` Manasi Navare 1 sibling, 1 reply; 10+ messages in thread From: Ville Syrjälä @ 2017-06-02 10:34 UTC (permalink / raw) To: Manasi Navare; +Cc: intel-gfx On Thu, Jun 01, 2017 at 05:51:27PM -0700, Manasi Navare 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@intel.com> > --- > 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 */ > + 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); > 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; This looks a bit questionable. What if eg. just the link_rate is out of bounds but the lane count is good? I think this could be solved in a different way with the patch I posted maybe one year ago that allowed rate_index() to return a non-exact match, and by clamping the 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; > > intel_dp->compliance.test_lane_count = test_lane_count; > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters 2017-06-02 10:34 ` Ville Syrjälä @ 2017-06-05 18:39 ` Manasi Navare 0 siblings, 0 replies; 10+ messages in thread From: Manasi Navare @ 2017-06-05 18:39 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Fri, Jun 02, 2017 at 01:34:10PM +0300, Ville Syrjälä wrote: > On Thu, Jun 01, 2017 at 05:51:27PM -0700, Manasi Navare 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@intel.com> > > --- > > 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 */ > > + 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); > > 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; > > This looks a bit questionable. What if eg. just the link_rate is out of > bounds but the lane count is good? > Hi Ville, Thanks for your feedback/concern here. But since the test requests both link rate and lane count to be used for the compliance run, even if one of the parameters are out of bound then we need to bail from using that combination in the compliance test run and instead go with the fallback values. Thought? Manasi > I think this could be solved in a different way with the patch I posted > maybe one year ago that allowed rate_index() to return a non-exact match, > and by clamping the 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; > > > > intel_dp->compliance.test_lane_count = test_lane_count; > > -- > > 2.1.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated 2017-06-02 0:51 [PATCH 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated Manasi Navare 2017-06-02 0:51 ` [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters Manasi Navare @ 2017-06-02 1:04 ` Patchwork 2017-06-02 8:22 ` [PATCH 1/2] " Jani Nikula 2 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2017-06-02 1:04 UTC (permalink / raw) To: Navare, Manasi D; +Cc: intel-gfx == Series Details == Series: series starting with [1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated URL : https://patchwork.freedesktop.org/series/25191/ State : success == Summary == Series 25191v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/25191/revisions/1/mbox/ Test kms_force_connector_basic: Subgroup force-connector-state: pass -> SKIP (fi-snb-2520m) fdo#101048 fdo#101048 https://bugs.freedesktop.org/show_bug.cgi?id=101048 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:447s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:440s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:576s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:508s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:492s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:482s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:432s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:411s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:419s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:488s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:472s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:465s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:574s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:463s fi-skl-6700hq total:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:433s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:468s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:495s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:436s fi-snb-2520m total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:531s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:401s 2c9abf8ec88cf4b3d3735976bb0ff7a4991946b2 drm-tip: 2017y-06m-01d-13h-29m-05s UTC integration manifest dc0322a drm/i915/dp: Validate the compliance test link parameters b121fbf drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4864/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated 2017-06-02 0:51 [PATCH 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated Manasi Navare 2017-06-02 0:51 ` [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters Manasi Navare 2017-06-02 1:04 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated Patchwork @ 2017-06-02 8:22 ` Jani Nikula 2 siblings, 0 replies; 10+ messages in thread From: Jani Nikula @ 2017-06-02 8:22 UTC (permalink / raw) To: Manasi Navare, intel-gfx On Fri, 02 Jun 2017, Manasi Navare <manasi.d.navare@intel.com> wrote: > This function now takes the link rate and lane ocunt to be validated > as an argument so that this can be used for validating even the > compliance test link parameters. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> Reviewed-by: Jani Nikula <jani.nikula@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 58dca87..832786d 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -322,19 +322,20 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp, > return 0; > } > > -static bool intel_dp_link_params_valid(struct intel_dp *intel_dp) > +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate, > + uint8_t lane_count) > { > /* > * FIXME: we need to synchronize the current link parameters with > * hardware readout. Currently fast link training doesn't work on > * boot-up. > */ > - if (intel_dp->link_rate == 0 || > - intel_dp->link_rate > intel_dp->max_link_rate) > + if (link_rate == 0 || > + link_rate > intel_dp->max_link_rate) > return false; > > - if (intel_dp->lane_count == 0 || > - intel_dp->lane_count > intel_dp_max_lane_count(intel_dp)) > + if (lane_count == 0 || > + lane_count > intel_dp_max_lane_count(intel_dp)) > return false; > > return true; > @@ -4260,7 +4261,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) > * Validate the cached values of intel_dp->link_rate and > * intel_dp->lane_count before attempting to retrain. > */ > - if (!intel_dp_link_params_valid(intel_dp)) > + if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate, > + intel_dp->lane_count)) > return; > > /* Retrain if Channel EQ or CR not ok */ -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-06-08 18:31 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-02 0:51 [PATCH 1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated Manasi Navare 2017-06-02 0:51 ` [PATCH 2/2] drm/i915/dp: Validate the compliance test link parameters Manasi Navare 2017-06-02 8:27 ` Jani Nikula 2017-06-02 8:55 ` Jani Nikula 2017-06-08 18:36 ` Manasi Navare 2017-06-05 18:27 ` Manasi Navare 2017-06-02 10:34 ` Ville Syrjälä 2017-06-05 18:39 ` Manasi Navare 2017-06-02 1:04 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/dp: Generalize intel_dp_link_params function to accept arguments to be validated Patchwork 2017-06-02 8:22 ` [PATCH 1/2] " Jani Nikula
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.