From: Imre Deak <imre.deak@intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 02/20] drm/i915/dp: Move link train params to a substruct in intel_dp
Date: Wed, 15 May 2024 16:26:50 +0300 [thread overview]
Message-ID: <ZkS4Gtgfha4hBndA@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <87h6ezck4u.fsf@intel.com>
On Wed, May 15, 2024 at 03:52:33PM +0300, Jani Nikula wrote:
> On Tue, 14 May 2024, Imre Deak <imre.deak@intel.com> wrote:
> > For clarity move the link training parameters updated during link
> > training based on the pass/fail LT result under a substruct in intel_dp.
> > This prepares for later patches in this patchset adding similar params
> > here. Rename intel_dp_reset_max_link_params() to
> > intel_dp_reset_link_train_params() to better reflect what state gets
> > reset.
>
> High level bikeshedding, why "link_train" instead of just "link"?
It was link training I've been thinking about and that it's important to
know which state gets updated during LT and gets reset after a hotplug.
> You could have three groups: source, sink and link.
Ok, makes sense and link_train could be instead a more generic link
container.
>
> BR,
> Jani.
>
>
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > .../drm/i915/display/intel_display_types.h | 12 ++++----
> > drivers/gpu/drm/i915/display/intel_dp.c | 30 +++++++++----------
> > 2 files changed, 22 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index fec3de25ea54e..7edb533758416 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1738,7 +1738,6 @@ struct intel_dp {
> > u8 lane_count;
> > u8 sink_count;
> > bool link_trained;
> > - bool reset_link_params;
> > bool use_max_params;
> > u8 dpcd[DP_RECEIVER_CAP_SIZE];
> > u8 psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> > @@ -1759,10 +1758,13 @@ struct intel_dp {
> > /* intersection of source and sink rates */
> > int num_common_rates;
> > int common_rates[DP_MAX_SUPPORTED_RATES];
> > - /* Max lane count for the current link */
> > - int max_link_lane_count;
> > - /* Max rate for the current link */
> > - int max_link_rate;
> > + struct {
> > + /* Max lane count for the current link */
> > + int max_lane_count;
> > + /* Max rate for the current link */
> > + int max_rate;
> > + } link_train;
> > + bool reset_link_params;
> > int mso_link_count;
> > int mso_pixel_overlap;
> > /* sink or branch descriptor */
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 6b8a94d0ca999..ffa627c63e048 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -372,13 +372,13 @@ int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
> >
> > int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> > {
> > - switch (intel_dp->max_link_lane_count) {
> > + switch (intel_dp->link_train.max_lane_count) {
> > case 1:
> > case 2:
> > case 4:
> > - return intel_dp->max_link_lane_count;
> > + return intel_dp->link_train.max_lane_count;
> > default:
> > - MISSING_CASE(intel_dp->max_link_lane_count);
> > + MISSING_CASE(intel_dp->link_train.max_lane_count);
> > return 1;
> > }
> > }
> > @@ -644,7 +644,7 @@ static bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
> > * boot-up.
> > */
> > if (link_rate == 0 ||
> > - link_rate > intel_dp->max_link_rate)
> > + link_rate > intel_dp->link_train.max_rate)
> > return false;
> >
> > if (lane_count == 0 ||
> > @@ -705,8 +705,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > "Retrying Link training for eDP with same parameters\n");
> > return 0;
> > }
> > - intel_dp->max_link_rate = intel_dp_common_rate(intel_dp, index - 1);
> > - intel_dp->max_link_lane_count = lane_count;
> > + intel_dp->link_train.max_rate = intel_dp_common_rate(intel_dp, index - 1);
> > + intel_dp->link_train.max_lane_count = lane_count;
> > } else if (lane_count > 1) {
> > if (intel_dp_is_edp(intel_dp) &&
> > !intel_dp_can_link_train_fallback_for_edp(intel_dp,
> > @@ -716,8 +716,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > "Retrying Link training for eDP with same parameters\n");
> > return 0;
> > }
> > - intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > - intel_dp->max_link_lane_count = lane_count >> 1;
> > + intel_dp->link_train.max_rate = intel_dp_max_common_rate(intel_dp);
> > + intel_dp->link_train.max_lane_count = lane_count >> 1;
> > } else {
> > drm_err(&i915->drm, "Link Training Unsuccessful\n");
> > return -1;
> > @@ -1382,7 +1382,7 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
> > {
> > int len;
> >
> > - len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->max_link_rate);
> > + len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->link_train.max_rate);
> >
> > return intel_dp_common_rate(intel_dp, len - 1);
> > }
> > @@ -3017,10 +3017,10 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
> > intel_dp->lane_count = lane_count;
> > }
> >
> > -static void intel_dp_reset_max_link_params(struct intel_dp *intel_dp)
> > +static void intel_dp_reset_link_train_params(struct intel_dp *intel_dp)
> > {
> > - intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > - intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > + intel_dp->link_train.max_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > + intel_dp->link_train.max_rate = intel_dp_max_common_rate(intel_dp);
> > }
> >
> > /* Enable backlight PWM and backlight PP control. */
> > @@ -3355,7 +3355,7 @@ void intel_dp_sync_state(struct intel_encoder *encoder,
> > intel_dp_tunnel_resume(intel_dp, crtc_state, dpcd_updated);
> >
> > if (crtc_state)
> > - intel_dp_reset_max_link_params(intel_dp);
> > + intel_dp_reset_link_train_params(intel_dp);
> > }
> >
> > bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
> > @@ -5888,7 +5888,7 @@ intel_dp_detect(struct drm_connector *connector,
> > * supports link training fallback params.
> > */
> > if (intel_dp->reset_link_params || intel_dp->is_mst) {
> > - intel_dp_reset_max_link_params(intel_dp);
> > + intel_dp_reset_link_train_params(intel_dp);
> > intel_dp->reset_link_params = false;
> > }
> >
> > @@ -6740,7 +6740,7 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
> >
> > intel_dp_set_source_rates(intel_dp);
> > intel_dp_set_common_rates(intel_dp);
> > - intel_dp_reset_max_link_params(intel_dp);
> > + intel_dp_reset_link_train_params(intel_dp);
> >
> > /* init MST on ports that can support it */
> > intel_dp_mst_encoder_init(dig_port,
>
> --
> Jani Nikula, Intel
next prev parent reply other threads:[~2024-05-15 13:26 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 19:13 [PATCH 00/20] drm/i915/dp_mst: Enable link training fallback Imre Deak
2024-05-14 19:13 ` [PATCH 01/20] drm/i915/dp_mst: Align TUs to avoid splitting symbols across MTPs Imre Deak
2024-05-14 19:14 ` [PATCH 02/20] drm/i915/dp: Move link train params to a substruct in intel_dp Imre Deak
2024-05-15 12:52 ` Jani Nikula
2024-05-15 13:26 ` Imre Deak [this message]
2024-05-14 19:14 ` [PATCH 03/20] drm/i915/dp: Move link train fallback to intel_dp_link_training.c Imre Deak
2024-05-14 19:14 ` [PATCH 04/20] drm/i915/dp: Sanitize intel_dp_get_link_train_fallback_values() Imre Deak
2024-05-15 13:05 ` Jani Nikula
2024-05-15 13:20 ` Imre Deak
2024-05-14 19:14 ` [PATCH 05/20] drm/i915: Factor out function to modeset commit a set of pipes Imre Deak
2024-05-14 19:14 ` [PATCH 06/20] drm/i915/dp: Use a commit modeset for link retraining MST links Imre Deak
2024-05-14 19:14 ` [PATCH 07/20] drm/i915/dp: Recheck link state after modeset Imre Deak
2024-05-14 20:26 ` [PATCH v2 " Imre Deak
2024-05-14 19:14 ` [PATCH 08/20] drm/i915/dp: Reduce link params only after retrying with unchanged params Imre Deak
2024-05-14 19:14 ` [PATCH 09/20] drm/i915/dp: Remove the modeset retry event's dependece on atomic state Imre Deak
2024-05-14 19:14 ` [PATCH 10/20] drm/i915/dp: Send a link training modeset-retry uevent to all MST connectors Imre Deak
2024-05-14 19:14 ` [PATCH 11/20] drm/i915/dp: Use check link state work in the hotplug handler Imre Deak
2024-05-14 19:14 ` [PATCH 12/20] drm/i915/dp: Use check link state work in the detect handler Imre Deak
2024-05-14 19:14 ` [PATCH 13/20] drm/i915/dp: Use check link state work in the HPD IRQ handler Imre Deak
2024-05-14 19:14 ` [PATCH 14/20] drm/i915/dp: Disable link retraining after the last fallback step Imre Deak
2024-05-14 19:14 ` [PATCH 15/20] drm/i915/dp_mst: Reset intel_dp->link_trained during disabling Imre Deak
2024-05-14 19:14 ` [PATCH 16/20] drm/i915/dp_mst: Enable link training fallback for MST Imre Deak
2024-05-14 19:14 ` [PATCH 17/20] drm/i915/dp: Add debugfs entries to set a target link rate/lane count Imre Deak
2024-05-15 13:09 ` Jani Nikula
2024-05-15 13:23 ` Imre Deak
2024-05-14 19:14 ` [PATCH 18/20] drm/i915/dp: Add debugfs entry to force link training failure Imre Deak
2024-05-14 19:14 ` [PATCH 19/20] drm/i915/dp: Add debugfs entry to force link retrain Imre Deak
2024-05-14 19:14 ` [PATCH 20/20] drm/i915/dp: Add debugfs entry for link training info Imre Deak
2024-05-14 19:39 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp_mst: Enable link training fallback Patchwork
2024-05-14 19:39 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-05-14 19:47 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-05-14 20:59 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp_mst: Enable link training fallback (rev2) Patchwork
2024-05-14 20:59 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-05-14 21:06 ` ✓ Fi.CI.BAT: success " Patchwork
2024-05-15 6:57 ` ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZkS4Gtgfha4hBndA@ideak-desk.fi.intel.com \
--to=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).