| Message ID | 1430203432-4460-3-git-send-email-mika.kahola@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 4/28/2015 12:13 PM, Mika Kahola wrote: > This patch adds DP link training optimization by reusing the > previously trained values. > > v2: > - rebase > > V3: > - rebase > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 15adafc..bb1a8d0 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3794,7 +3794,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > intel_dp->DP = DP; > > if (channel_eq) { > - intel_dp->train_set_valid = is_edp(intel_dp); > + intel_dp->train_set_valid = true; > DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); > } > } any reason why this is split into a separate patch ?
On Tue, 2015-04-28 at 13:19 +0530, Sivakumar Thulasimani wrote: > > On 4/28/2015 12:13 PM, Mika Kahola wrote: > > This patch adds DP link training optimization by reusing the > > previously trained values. > > > > v2: > > - rebase > > > > V3: > > - rebase > > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 15adafc..bb1a8d0 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -3794,7 +3794,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > > intel_dp->DP = DP; > > > > if (channel_eq) { > > - intel_dp->train_set_valid = is_edp(intel_dp); > > + intel_dp->train_set_valid = true; > > DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); > > } > > } > any reason why this is split into a separate patch ? I discussed this with Jani and we agreed that one option would be to apply this optimization only for the eDP case. This second patch is for the possibility to cover the general DP. -Mika-
On 4/28/2015 1:44 PM, Mika Kahola wrote: > On Tue, 2015-04-28 at 13:19 +0530, Sivakumar Thulasimani wrote: >> On 4/28/2015 12:13 PM, Mika Kahola wrote: >>> This patch adds DP link training optimization by reusing the >>> previously trained values. >>> >>> v2: >>> - rebase >>> >>> V3: >>> - rebase >>> >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>> index 15adafc..bb1a8d0 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -3794,7 +3794,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) >>> intel_dp->DP = DP; >>> >>> if (channel_eq) { >>> - intel_dp->train_set_valid = is_edp(intel_dp); >>> + intel_dp->train_set_valid = true; >>> DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); >>> } >>> } >> any reason why this is split into a separate patch ? > I discussed this with Jani and we agreed that one option would be to > apply this optimization only for the eDP case. This second patch is for > the possibility to cover the general DP. > > -Mika- > > might be a nit pick so up to you to consider this or not :). since DP is part of the second patch, better to reset "train_set_valid" for long pulse HPD in the second patch as well. HPD is not enabled for eDP as of now.
On Tue, 2015-04-28 at 13:51 +0530, Sivakumar Thulasimani wrote: > > > On 4/28/2015 1:44 PM, Mika Kahola wrote: > > On Tue, 2015-04-28 at 13:19 +0530, Sivakumar Thulasimani wrote: > >> On 4/28/2015 12:13 PM, Mika Kahola wrote: > >>> This patch adds DP link training optimization by reusing the > >>> previously trained values. > >>> > >>> v2: > >>> - rebase > >>> > >>> V3: > >>> - rebase > >>> > >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com> > >>> --- > >>> drivers/gpu/drm/i915/intel_dp.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > >>> index 15adafc..bb1a8d0 100644 > >>> --- a/drivers/gpu/drm/i915/intel_dp.c > >>> +++ b/drivers/gpu/drm/i915/intel_dp.c > >>> @@ -3794,7 +3794,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > >>> intel_dp->DP = DP; > >>> > >>> if (channel_eq) { > >>> - intel_dp->train_set_valid = is_edp(intel_dp); > >>> + intel_dp->train_set_valid = true; > >>> DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); > >>> } > >>> } > >> any reason why this is split into a separate patch ? > > I discussed this with Jani and we agreed that one option would be to > > apply this optimization only for the eDP case. This second patch is for > > the possibility to cover the general DP. > > > > -Mika- > > > > > might be a nit pick so up to you to consider this or not :). since DP is > part of the second patch, better to reset "train_set_valid" for long > pulse HPD in the second patch as well. HPD is not enabled for eDP as of now. > So, you mean that the second patch could be written as independent of the first patch? In that way you could choose which patch you apply and you wouldn't have to apply the both patches to get the DP case covered.
On 4/28/2015 3:12 PM, Mika Kahola wrote: > On Tue, 2015-04-28 at 13:51 +0530, Sivakumar Thulasimani wrote: >> >> On 4/28/2015 1:44 PM, Mika Kahola wrote: >>> On Tue, 2015-04-28 at 13:19 +0530, Sivakumar Thulasimani wrote: >>>> On 4/28/2015 12:13 PM, Mika Kahola wrote: >>>>> This patch adds DP link training optimization by reusing the >>>>> previously trained values. >>>>> >>>>> v2: >>>>> - rebase >>>>> >>>>> V3: >>>>> - rebase >>>>> >>>>> Signed-off-by: Mika Kahola <mika.kahola@intel.com> >>>>> --- >>>>> drivers/gpu/drm/i915/intel_dp.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>>>> index 15adafc..bb1a8d0 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_dp.c >>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>>>> @@ -3794,7 +3794,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) >>>>> intel_dp->DP = DP; >>>>> >>>>> if (channel_eq) { >>>>> - intel_dp->train_set_valid = is_edp(intel_dp); >>>>> + intel_dp->train_set_valid = true; >>>>> DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); >>>>> } >>>>> } >>>> any reason why this is split into a separate patch ? >>> I discussed this with Jani and we agreed that one option would be to >>> apply this optimization only for the eDP case. This second patch is for >>> the possibility to cover the general DP. >>> >>> -Mika- >>> >>> >> might be a nit pick so up to you to consider this or not :). since DP is >> part of the second patch, better to reset "train_set_valid" for long >> pulse HPD in the second patch as well. HPD is not enabled for eDP as of now. >> > So, you mean that the second patch could be written as independent of > the first patch? In that way you could choose which patch you apply and > you wouldn't have to apply the both patches to get the DP case covered. > > nope, i was referring to the following change alone. @@ -4822,6 +4843,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) intel_display_power_get(dev_priv, power_domain); if (long_hpd) { + /* indicate that we need to restart link training */ + intel_dp->train_set_valid = false; to be specific, if 2nd patch is to enable this optimization for DP, the above change will belong here as well. eDP never has HPD enabled so it will never be hit for eDP only scenario.
On Tue, 2015-04-28 at 15:18 +0530, Sivakumar Thulasimani wrote: > On 4/28/2015 3:12 PM, Mika Kahola wrote: > > On Tue, 2015-04-28 at 13:51 +0530, Sivakumar Thulasimani wrote: > >> > >> On 4/28/2015 1:44 PM, Mika Kahola wrote: > >>> On Tue, 2015-04-28 at 13:19 +0530, Sivakumar Thulasimani wrote: > >>>> On 4/28/2015 12:13 PM, Mika Kahola wrote: > >>>>> This patch adds DP link training optimization by reusing the > >>>>> previously trained values. > >>>>> > >>>>> v2: > >>>>> - rebase > >>>>> > >>>>> V3: > >>>>> - rebase > >>>>> > >>>>> Signed-off-by: Mika Kahola <mika.kahola@intel.com> > >>>>> --- > >>>>> drivers/gpu/drm/i915/intel_dp.c | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > >>>>> index 15adafc..bb1a8d0 100644 > >>>>> --- a/drivers/gpu/drm/i915/intel_dp.c > >>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c > >>>>> @@ -3794,7 +3794,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > >>>>> intel_dp->DP = DP; > >>>>> > >>>>> if (channel_eq) { > >>>>> - intel_dp->train_set_valid = is_edp(intel_dp); > >>>>> + intel_dp->train_set_valid = true; > >>>>> DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); > >>>>> } > >>>>> } > >>>> any reason why this is split into a separate patch ? > >>> I discussed this with Jani and we agreed that one option would be to > >>> apply this optimization only for the eDP case. This second patch is for > >>> the possibility to cover the general DP. > >>> > >>> -Mika- > >>> > >>> > >> might be a nit pick so up to you to consider this or not :). since DP is > >> part of the second patch, better to reset "train_set_valid" for long > >> pulse HPD in the second patch as well. HPD is not enabled for eDP as of now. > >> > > So, you mean that the second patch could be written as independent of > > the first patch? In that way you could choose which patch you apply and > > you wouldn't have to apply the both patches to get the DP case covered. > > > > > nope, i was referring to the following change alone. > > @@ -4822,6 +4843,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > intel_display_power_get(dev_priv, power_domain); > > if (long_hpd) { > + /* indicate that we need to restart link training */ > + intel_dp->train_set_valid = false; > > > to be specific, if 2nd patch is to enable this optimization for DP, the above change will belong here as well. eDP never has HPD enabled so it will never be hit for eDP only scenario. > Ok, now I'm following you. I rephrase the patch set so the HPD is taken into consideration on this second patch as well.
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6274
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 301/301 301/301
SNB 318/318 318/318
IVB 345/345 345/345
BYT -1 285/285 284/285
BDW 321/321 321/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@kms_setmode@invalid-clone-exclusive-crtc PASS(7) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
Note: You need to pay more attention to line start with '*'
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 15adafc..bb1a8d0 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3794,7 +3794,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) intel_dp->DP = DP; if (channel_eq) { - intel_dp->train_set_valid = is_edp(intel_dp); + intel_dp->train_set_valid = true; DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n"); } }
This patch adds DP link training optimization by reusing the previously trained values. v2: - rebase V3: - rebase Signed-off-by: Mika Kahola <mika.kahola@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)