From: Sean Anderson <sean.anderson@linux.dev> To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Cc: Michal Simek <michal.simek@amd.com>, David Airlie <airlied@gmail.com>, linux-kernel@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>, linux-arm-kernel@lists.infradead.org, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v2 3/8] drm: zynqmp_dp: Adjust training values per-lane Date: Thu, 21 Mar 2024 11:35:14 -0400 [thread overview] Message-ID: <f6e5931a-22ab-4291-b1c5-9885be0b1b22@linux.dev> (raw) In-Reply-To: <c16665d3-ff3a-48d7-9f4e-076473a3b45d@ideasonboard.com> On 3/20/24 01:57, Tomi Valkeinen wrote: > On 20/03/2024 00:51, Sean Anderson wrote: >> The feedback we get from the DPRX is per-lane. Make changes using this >> information, instead of picking the maximum values from all lanes. This >> results in more-consistent training on marginal links. >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> --- >> >> (no changes since v1) >> >> drivers/gpu/drm/xlnx/zynqmp_dp.c | 23 ++++++++--------------- >> 1 file changed, 8 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c >> index 98a32e6a0459..8635b5673386 100644 >> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c >> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c >> @@ -605,28 +605,21 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp, >> u8 link_status[DP_LINK_STATUS_SIZE]) >> { >> u8 *train_set = dp->train_set; >> - u8 voltage = 0, preemphasis = 0; >> u8 i; >> for (i = 0; i < dp->mode.lane_cnt; i++) { >> - u8 v = drm_dp_get_adjust_request_voltage(link_status, i); >> - u8 p = drm_dp_get_adjust_request_pre_emphasis(link_status, i); >> + u8 voltage = drm_dp_get_adjust_request_voltage(link_status, i); >> + u8 preemphasis = >> + drm_dp_get_adjust_request_pre_emphasis(link_status, i); >> - if (v > voltage) >> - voltage = v; >> + if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) >> + voltage |= DP_TRAIN_MAX_SWING_REACHED; >> - if (p > preemphasis) >> - preemphasis = p; >> - } >> + if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) >> + preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; >> - if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) >> - voltage |= DP_TRAIN_MAX_SWING_REACHED; >> - >> - if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) >> - preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; >> - >> - for (i = 0; i < dp->mode.lane_cnt; i++) >> train_set[i] = voltage | preemphasis; >> + } >> } >> /** > > Looks fine to me, but a few cosmetic suggestions, feel free to ignore if not to your liking: > > 1) > > u8 voltage, preemphasis; > > voltage = drm_dp_get_adjust_request_voltage(link_status, i); > preemphasis = drm_dp_get_adjust_request_pre_emphasis(link_status, i); If the comment here is about the line break, I agree that this looks better but the second line is over 80 characters. > 2) > > for (unsigned int i = 0; i < dp->mode.lane_cnt; i++) Is this allowed now? > 3) > > dp->train_set[i] = voltage | preemphasis; This would be undone in patch 7/8. --Sean > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > Tomi >
WARNING: multiple messages have this Message-ID (diff)
From: Sean Anderson <sean.anderson@linux.dev> To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Cc: Michal Simek <michal.simek@amd.com>, David Airlie <airlied@gmail.com>, linux-kernel@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>, linux-arm-kernel@lists.infradead.org, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v2 3/8] drm: zynqmp_dp: Adjust training values per-lane Date: Thu, 21 Mar 2024 11:35:14 -0400 [thread overview] Message-ID: <f6e5931a-22ab-4291-b1c5-9885be0b1b22@linux.dev> (raw) In-Reply-To: <c16665d3-ff3a-48d7-9f4e-076473a3b45d@ideasonboard.com> On 3/20/24 01:57, Tomi Valkeinen wrote: > On 20/03/2024 00:51, Sean Anderson wrote: >> The feedback we get from the DPRX is per-lane. Make changes using this >> information, instead of picking the maximum values from all lanes. This >> results in more-consistent training on marginal links. >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> --- >> >> (no changes since v1) >> >> drivers/gpu/drm/xlnx/zynqmp_dp.c | 23 ++++++++--------------- >> 1 file changed, 8 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c >> index 98a32e6a0459..8635b5673386 100644 >> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c >> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c >> @@ -605,28 +605,21 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp, >> u8 link_status[DP_LINK_STATUS_SIZE]) >> { >> u8 *train_set = dp->train_set; >> - u8 voltage = 0, preemphasis = 0; >> u8 i; >> for (i = 0; i < dp->mode.lane_cnt; i++) { >> - u8 v = drm_dp_get_adjust_request_voltage(link_status, i); >> - u8 p = drm_dp_get_adjust_request_pre_emphasis(link_status, i); >> + u8 voltage = drm_dp_get_adjust_request_voltage(link_status, i); >> + u8 preemphasis = >> + drm_dp_get_adjust_request_pre_emphasis(link_status, i); >> - if (v > voltage) >> - voltage = v; >> + if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) >> + voltage |= DP_TRAIN_MAX_SWING_REACHED; >> - if (p > preemphasis) >> - preemphasis = p; >> - } >> + if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) >> + preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; >> - if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3) >> - voltage |= DP_TRAIN_MAX_SWING_REACHED; >> - >> - if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2) >> - preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED; >> - >> - for (i = 0; i < dp->mode.lane_cnt; i++) >> train_set[i] = voltage | preemphasis; >> + } >> } >> /** > > Looks fine to me, but a few cosmetic suggestions, feel free to ignore if not to your liking: > > 1) > > u8 voltage, preemphasis; > > voltage = drm_dp_get_adjust_request_voltage(link_status, i); > preemphasis = drm_dp_get_adjust_request_pre_emphasis(link_status, i); If the comment here is about the line break, I agree that this looks better but the second line is over 80 characters. > 2) > > for (unsigned int i = 0; i < dp->mode.lane_cnt; i++) Is this allowed now? > 3) > > dp->train_set[i] = voltage | preemphasis; This would be undone in patch 7/8. --Sean > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > Tomi > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-03-21 15:35 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-19 22:51 [PATCH v2 0/8] drm: zynqmp_dp: Misc. patches and debugfs support Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-19 22:51 ` [PATCH v2 1/8] drm: xlnx: Fix kerneldoc Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-20 5:42 ` Tomi Valkeinen 2024-03-20 5:42 ` Tomi Valkeinen 2024-03-20 6:05 ` Randy Dunlap 2024-03-20 6:05 ` Randy Dunlap 2024-03-21 15:33 ` Sean Anderson 2024-03-21 15:33 ` Sean Anderson 2024-03-22 5:50 ` Tomi Valkeinen 2024-03-22 5:50 ` Tomi Valkeinen 2024-03-22 15:22 ` Sean Anderson 2024-03-22 15:22 ` Sean Anderson 2024-03-19 22:51 ` [PATCH v2 2/8] drm: zynqmp_dp: Downgrade log level for aux retries message Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-20 5:46 ` Tomi Valkeinen 2024-03-20 5:46 ` Tomi Valkeinen 2024-03-19 22:51 ` [PATCH v2 3/8] drm: zynqmp_dp: Adjust training values per-lane Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-20 5:57 ` Tomi Valkeinen 2024-03-20 5:57 ` Tomi Valkeinen 2024-03-21 15:35 ` Sean Anderson [this message] 2024-03-21 15:35 ` Sean Anderson 2024-03-19 22:51 ` [PATCH v2 4/8] drm: zynqmp_dp: Rearrange zynqmp_dp for better padding Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-20 6:14 ` Tomi Valkeinen 2024-03-20 6:14 ` Tomi Valkeinen 2024-03-21 15:43 ` Sean Anderson 2024-03-21 15:43 ` Sean Anderson 2024-03-19 22:51 ` [PATCH v2 5/8] drm: zynqmp_dp: Don't retrain the link in our IRQ Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-20 6:53 ` Tomi Valkeinen 2024-03-20 6:53 ` Tomi Valkeinen 2024-03-21 15:52 ` Sean Anderson 2024-03-21 15:52 ` Sean Anderson 2024-03-21 17:25 ` Tomi Valkeinen 2024-03-21 17:25 ` Tomi Valkeinen 2024-03-21 18:01 ` Sean Anderson 2024-03-21 18:01 ` Sean Anderson 2024-03-21 19:08 ` Tomi Valkeinen 2024-03-21 19:08 ` Tomi Valkeinen 2024-03-21 19:17 ` Sean Anderson 2024-03-21 19:17 ` Sean Anderson 2024-03-22 5:32 ` Tomi Valkeinen 2024-03-22 5:32 ` Tomi Valkeinen 2024-03-22 16:18 ` Sean Anderson 2024-03-22 16:18 ` Sean Anderson 2024-03-22 18:09 ` Tomi Valkeinen 2024-03-22 18:09 ` Tomi Valkeinen 2024-03-22 21:22 ` Sean Anderson 2024-03-22 21:22 ` Sean Anderson 2024-03-23 8:54 ` Tomi Valkeinen 2024-03-23 8:54 ` Tomi Valkeinen 2024-03-19 22:51 ` [PATCH v2 6/8] drm: zynqmp_dp: Add locking Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-19 22:51 ` [PATCH v2 7/8] drm: zynqmp_dp: Split off several helper functions Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-20 7:36 ` Tomi Valkeinen 2024-03-20 7:36 ` Tomi Valkeinen 2024-03-19 22:51 ` [PATCH v2 8/8] drm: zynqmp_dp: Add debugfs interface for compliance testing Sean Anderson 2024-03-19 22:51 ` Sean Anderson 2024-03-20 7:49 ` Tomi Valkeinen 2024-03-20 7:49 ` Tomi Valkeinen 2024-03-21 16:08 ` Sean Anderson 2024-03-21 16:08 ` Sean Anderson 2024-03-21 16:31 ` Tomi Valkeinen 2024-03-21 16:31 ` Tomi Valkeinen 2024-03-21 16:35 ` Sean Anderson 2024-03-21 16:35 ` Sean Anderson 2024-03-19 23:02 ` [PATCH v2 0/8] drm: zynqmp_dp: Misc. patches and debugfs support Sean Anderson 2024-03-19 23:02 ` Sean Anderson
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=f6e5931a-22ab-4291-b1c5-9885be0b1b22@linux.dev \ --to=sean.anderson@linux.dev \ --cc=airlied@gmail.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maarten.lankhorst@linux.intel.com \ --cc=michal.simek@amd.com \ --cc=mripard@kernel.org \ --cc=tomi.valkeinen@ideasonboard.com \ --cc=tzimmermann@suse.de \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.