| Message ID | 20191003150653.15881-3-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DP Phy compliace auto test. | expand |
On Thu, Oct 03, 2019 at 08:36:49PM +0530, Animesh Manna wrote: > vswing/pre-emphasis adjustment calculation is needed in processing > of auto phy compliance request other than link training, so moved > the same function in intel_dp.c. > > No functional change. You could just make it a non static function instead of moving to intel_dp.c Manasi > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 32 +++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_dp.h | 3 ++ > .../drm/i915/display/intel_dp_link_training.c | 32 ------------------- > 3 files changed, 35 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 1aa39e92f0df..7d33e20dfc87 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4026,6 +4026,38 @@ ivb_cpu_edp_signal_levels(u8 train_set) > } > } > > +void > +intel_get_adjust_train(struct intel_dp *intel_dp, > + const u8 *link_status) > +{ > + u8 v = 0; > + u8 p = 0; > + int lane; > + u8 voltage_max; > + u8 preemph_max; > + > + for (lane = 0; lane < intel_dp->lane_count; lane++) { > + u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane); > + u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane); > + > + if (this_v > v) > + v = this_v; > + if (this_p > p) > + p = this_p; > + } > + > + voltage_max = intel_dp_voltage_max(intel_dp); > + if (v >= voltage_max) > + v = voltage_max | DP_TRAIN_MAX_SWING_REACHED; > + > + preemph_max = intel_dp_pre_emphasis_max(intel_dp, v); > + if (p >= preemph_max) > + p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; > + > + for (lane = 0; lane < 4; lane++) > + intel_dp->train_set[lane] = v | p; > +} > + > void > intel_dp_set_signal_levels(struct intel_dp *intel_dp) > { > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h > index a194b5b6da05..8f8333afd43d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.h > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > @@ -91,6 +91,9 @@ void > intel_dp_program_link_training_pattern(struct intel_dp *intel_dp, > u8 dp_train_pat); > void > +intel_get_adjust_train(struct intel_dp *intel_dp, > + const u8 *link_status); > +void > intel_dp_set_signal_levels(struct intel_dp *intel_dp); > void intel_dp_set_idle_link_train(struct intel_dp *intel_dp); > u8 > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > index 2a1130dd1ad0..1e38584e7d56 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > @@ -34,38 +34,6 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE]) > link_status[3], link_status[4], link_status[5]); > } > > -static void > -intel_get_adjust_train(struct intel_dp *intel_dp, > - const u8 link_status[DP_LINK_STATUS_SIZE]) > -{ > - u8 v = 0; > - u8 p = 0; > - int lane; > - u8 voltage_max; > - u8 preemph_max; > - > - for (lane = 0; lane < intel_dp->lane_count; lane++) { > - u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane); > - u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane); > - > - if (this_v > v) > - v = this_v; > - if (this_p > p) > - p = this_p; > - } > - > - voltage_max = intel_dp_voltage_max(intel_dp); > - if (v >= voltage_max) > - v = voltage_max | DP_TRAIN_MAX_SWING_REACHED; > - > - preemph_max = intel_dp_pre_emphasis_max(intel_dp, v); > - if (p >= preemph_max) > - p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; > - > - for (lane = 0; lane < 4; lane++) > - intel_dp->train_set[lane] = v | p; > -} > - > static bool > intel_dp_set_link_train(struct intel_dp *intel_dp, > u8 dp_train_pat) > -- > 2.22.0 >
On 10/22/2019 4:27 AM, Manasi Navare wrote: > On Thu, Oct 03, 2019 at 08:36:49PM +0530, Animesh Manna wrote: >> vswing/pre-emphasis adjustment calculation is needed in processing >> of auto phy compliance request other than link training, so moved >> the same function in intel_dp.c. >> >> No functional change. > You could just make it a non static function instead of moving to intel_dp.c Initially I did the same ... :) Later I thought intel_dp_link_training.c file is mainly focused only on link training process, start_link_train() and stop_link_train() are only exposed. I thought I may not be allowed to expose intel_get_adjust_train() as it is not exclusive for link training. Please let me know your view. Regards, Animesh
On Tue, Oct 22, 2019 at 07:34:13PM +0530, Animesh Manna wrote: > > On 10/22/2019 4:27 AM, Manasi Navare wrote: > >On Thu, Oct 03, 2019 at 08:36:49PM +0530, Animesh Manna wrote: > >>vswing/pre-emphasis adjustment calculation is needed in processing > >>of auto phy compliance request other than link training, so moved > >>the same function in intel_dp.c. > >> > >>No functional change. > >You could just make it a non static function instead of moving to intel_dp.c > > Initially I did the same ... :) > Later I thought intel_dp_link_training.c file is mainly focused only on link > training process, start_link_train() and stop_link_train() are only exposed. > I thought I may not be allowed to expose intel_get_adjust_train() as it is > not exclusive for link training. > Please let me know your view. Infact now that I look at this, i think the prepare_phy_test function should only probably get the drm_get_phy_test_pattern and populate in the intel_dp_compliance structure and then adjust train and all that can happen right before pattern update which will need to happen in atomic_commit_tail. How do you handle the regular link training sequence in the case of PHY test? What happens to the adjust train the the regular link training clock recovery and channel eq stages? Manasi > > Regards, > Animesh
On 10/22/2019 11:10 PM, Manasi Navare wrote: > On Tue, Oct 22, 2019 at 07:34:13PM +0530, Animesh Manna wrote: >> On 10/22/2019 4:27 AM, Manasi Navare wrote: >>> On Thu, Oct 03, 2019 at 08:36:49PM +0530, Animesh Manna wrote: >>>> vswing/pre-emphasis adjustment calculation is needed in processing >>>> of auto phy compliance request other than link training, so moved >>>> the same function in intel_dp.c. >>>> >>>> No functional change. >>> You could just make it a non static function instead of moving to intel_dp.c >> Initially I did the same ... :) >> Later I thought intel_dp_link_training.c file is mainly focused only on link >> training process, start_link_train() and stop_link_train() are only exposed. >> I thought I may not be allowed to expose intel_get_adjust_train() as it is >> not exclusive for link training. >> Please let me know your view. > Infact now that I look at this, i think the prepare_phy_test function should only > probably get the drm_get_phy_test_pattern and populate in the intel_dp_compliance > structure and then adjust train and all that can happen right before pattern update > which will need to happen in atomic_commit_tail. Okay, in commit-tail we follow modeset sequence and use encoder specific hook to do encoder specific programming. Can you please share your thought about the right place to do DP phy compliance specific programming. > > How do you handle the regular link training sequence in the case of PHY test? In automated phy compliance testing as such there is no requirement to do link training in between. Do you see any need of link training in between compliance testing? > What happens to the adjust train the the regular link training clock recovery and > channel eq stages? I felt adjust train calculation can be exposed from intel_dp.c/.h and same like other helper link training process(clock recovery + channel eq) can use it. Regards, Animesh > > Manasi > >> Regards, >> Animesh
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 1aa39e92f0df..7d33e20dfc87 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4026,6 +4026,38 @@ ivb_cpu_edp_signal_levels(u8 train_set) } } +void +intel_get_adjust_train(struct intel_dp *intel_dp, + const u8 *link_status) +{ + u8 v = 0; + u8 p = 0; + int lane; + u8 voltage_max; + u8 preemph_max; + + for (lane = 0; lane < intel_dp->lane_count; lane++) { + u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane); + u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane); + + if (this_v > v) + v = this_v; + if (this_p > p) + p = this_p; + } + + voltage_max = intel_dp_voltage_max(intel_dp); + if (v >= voltage_max) + v = voltage_max | DP_TRAIN_MAX_SWING_REACHED; + + preemph_max = intel_dp_pre_emphasis_max(intel_dp, v); + if (p >= preemph_max) + p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; + + for (lane = 0; lane < 4; lane++) + intel_dp->train_set[lane] = v | p; +} + void intel_dp_set_signal_levels(struct intel_dp *intel_dp) { diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h index a194b5b6da05..8f8333afd43d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.h +++ b/drivers/gpu/drm/i915/display/intel_dp.h @@ -91,6 +91,9 @@ void intel_dp_program_link_training_pattern(struct intel_dp *intel_dp, u8 dp_train_pat); void +intel_get_adjust_train(struct intel_dp *intel_dp, + const u8 *link_status); +void intel_dp_set_signal_levels(struct intel_dp *intel_dp); void intel_dp_set_idle_link_train(struct intel_dp *intel_dp); u8 diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c index 2a1130dd1ad0..1e38584e7d56 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c @@ -34,38 +34,6 @@ intel_dp_dump_link_status(const u8 link_status[DP_LINK_STATUS_SIZE]) link_status[3], link_status[4], link_status[5]); } -static void -intel_get_adjust_train(struct intel_dp *intel_dp, - const u8 link_status[DP_LINK_STATUS_SIZE]) -{ - u8 v = 0; - u8 p = 0; - int lane; - u8 voltage_max; - u8 preemph_max; - - for (lane = 0; lane < intel_dp->lane_count; lane++) { - u8 this_v = drm_dp_get_adjust_request_voltage(link_status, lane); - u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status, lane); - - if (this_v > v) - v = this_v; - if (this_p > p) - p = this_p; - } - - voltage_max = intel_dp_voltage_max(intel_dp); - if (v >= voltage_max) - v = voltage_max | DP_TRAIN_MAX_SWING_REACHED; - - preemph_max = intel_dp_pre_emphasis_max(intel_dp, v); - if (p >= preemph_max) - p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; - - for (lane = 0; lane < 4; lane++) - intel_dp->train_set[lane] = v | p; -} - static bool intel_dp_set_link_train(struct intel_dp *intel_dp, u8 dp_train_pat)
vswing/pre-emphasis adjustment calculation is needed in processing of auto phy compliance request other than link training, so moved the same function in intel_dp.c. No functional change. Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 32 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_dp.h | 3 ++ .../drm/i915/display/intel_dp_link_training.c | 32 ------------------- 3 files changed, 35 insertions(+), 32 deletions(-)