From: "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com>
To: Ander Conselvan de Oliveira
<ander.conselvan.de.oliveira@intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2] drm/i915: Don't pass *DP around to link training functions
Date: Wed, 21 Oct 2015 19:38:14 +0530 [thread overview]
Message-ID: <56279C4E.90301@intel.com> (raw)
In-Reply-To: <1445435529-19511-1-git-send-email-ander.conselvan.de.oliveira@intel.com>
On 10/21/2015 7:22 PM, Ander Conselvan de Oliveira wrote:
> It just makes the code more confusing, so just reference intel_dp->DP
> directly.
>
> Note that this also fix a bug where the value of intel_dp->DP could be
> different than the last value written to the hw, due to an early return
> that would skip the 'intel_dp->DP = DP' line.
>
> v2: Don't preserve old DP value on failure. (Sivakumar)
> - Don't call drm_dp_clock_recovery_ok() twice. (Sivakumar)
> - Keep return type of clock recovery and channel equalization
> functions as void. (Ander)
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 43 +++++++++++++++++------------------------
> 1 file changed, 18 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8287df4..f310dab 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3596,7 +3596,6 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
>
> static bool
> intel_dp_set_link_train(struct intel_dp *intel_dp,
> - uint32_t *DP,
> uint8_t dp_train_pat)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -3605,9 +3604,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> uint8_t buf[sizeof(intel_dp->train_set) + 1];
> int ret, len;
>
> - _intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
> + _intel_dp_set_link_train(intel_dp, &intel_dp->DP, dp_train_pat);
>
> - I915_WRITE(intel_dp->output_reg, *DP);
> + I915_WRITE(intel_dp->output_reg, intel_dp->DP);
> POSTING_READ(intel_dp->output_reg);
>
> buf[0] = dp_train_pat;
> @@ -3628,17 +3627,17 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> }
>
> static bool
> -intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP,
> +intel_dp_reset_link_train(struct intel_dp *intel_dp,
> uint8_t dp_train_pat)
> {
> if (!intel_dp->train_set_valid)
> memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> - intel_dp_set_signal_levels(intel_dp, DP);
> - return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
> + intel_dp_set_signal_levels(intel_dp, &intel_dp->DP);
can this be removed as well ? we are already passing intel_dp so same as
other places.
> + return intel_dp_set_link_train(intel_dp, dp_train_pat);
> }
>
> static bool
> -intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
> +intel_dp_update_link_train(struct intel_dp *intel_dp,
> const uint8_t link_status[DP_LINK_STATUS_SIZE])
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -3647,9 +3646,9 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
> int ret;
>
> intel_get_adjust_train(intel_dp, link_status);
> - intel_dp_set_signal_levels(intel_dp, DP);
> + intel_dp_set_signal_levels(intel_dp, &intel_dp->DP);
>
> - I915_WRITE(intel_dp->output_reg, *DP);
> + I915_WRITE(intel_dp->output_reg, intel_dp->DP);
> POSTING_READ(intel_dp->output_reg);
>
> ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
> @@ -3698,7 +3697,6 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> int i;
> uint8_t voltage;
> int voltage_tries, loop_tries;
> - uint32_t DP = intel_dp->DP;
> uint8_t link_config[2];
> uint8_t link_bw, rate_select;
>
> @@ -3722,10 +3720,10 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> link_config[1] = DP_SET_ANSI_8B10B;
> drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
>
> - DP |= DP_PORT_EN;
> + intel_dp->DP |= DP_PORT_EN;
>
> /* clock recovery */
> - if (!intel_dp_reset_link_train(intel_dp, &DP,
> + if (!intel_dp_reset_link_train(intel_dp,
> DP_TRAINING_PATTERN_1 |
> DP_LINK_SCRAMBLING_DISABLE)) {
> DRM_ERROR("failed to enable link training\n");
> @@ -3757,7 +3755,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> DRM_DEBUG_KMS("clock recovery not ok, reset");
> /* clear the flag as we are not reusing train set */
> intel_dp->train_set_valid = false;
> - if (!intel_dp_reset_link_train(intel_dp, &DP,
> + if (!intel_dp_reset_link_train(intel_dp,
> DP_TRAINING_PATTERN_1 |
> DP_LINK_SCRAMBLING_DISABLE)) {
> DRM_ERROR("failed to enable link training\n");
> @@ -3776,7 +3774,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> DRM_ERROR("too many full retries, give up\n");
> break;
> }
> - intel_dp_reset_link_train(intel_dp, &DP,
> + intel_dp_reset_link_train(intel_dp,
> DP_TRAINING_PATTERN_1 |
> DP_LINK_SCRAMBLING_DISABLE);
> voltage_tries = 0;
> @@ -3795,13 +3793,11 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
>
> /* Update training set as requested by target */
> - if (!intel_dp_update_link_train(intel_dp, &DP, link_status)) {
> + if (!intel_dp_update_link_train(intel_dp, link_status)) {
> DRM_ERROR("failed to update link training\n");
> break;
> }
> }
> -
> - intel_dp->DP = DP;
> }
>
> static void
> @@ -3811,7 +3807,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> struct drm_device *dev = dig_port->base.base.dev;
> bool channel_eq = false;
> int tries, cr_tries;
> - uint32_t DP = intel_dp->DP;
> uint32_t training_pattern = DP_TRAINING_PATTERN_2;
>
> /*
> @@ -3830,7 +3825,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
>
> /* channel equalization */
> - if (!intel_dp_set_link_train(intel_dp, &DP,
> + if (!intel_dp_set_link_train(intel_dp,
> training_pattern |
> DP_LINK_SCRAMBLING_DISABLE)) {
> DRM_ERROR("failed to start channel equalization\n");
> @@ -3859,7 +3854,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> intel_dp->lane_count)) {
> intel_dp->train_set_valid = false;
> intel_dp_link_training_clock_recovery(intel_dp);
> - intel_dp_set_link_train(intel_dp, &DP,
> + intel_dp_set_link_train(intel_dp,
> training_pattern |
> DP_LINK_SCRAMBLING_DISABLE);
> cr_tries++;
> @@ -3876,7 +3871,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> if (tries > 5) {
> intel_dp->train_set_valid = false;
> intel_dp_link_training_clock_recovery(intel_dp);
> - intel_dp_set_link_train(intel_dp, &DP,
> + intel_dp_set_link_train(intel_dp,
> training_pattern |
> DP_LINK_SCRAMBLING_DISABLE);
> tries = 0;
> @@ -3885,7 +3880,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> }
>
> /* Update training set as requested by target */
> - if (!intel_dp_update_link_train(intel_dp, &DP, link_status)) {
> + if (!intel_dp_update_link_train(intel_dp, link_status)) {
> DRM_ERROR("failed to update link training\n");
> break;
> }
> @@ -3894,8 +3889,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
>
> intel_dp_set_idle_link_train(intel_dp);
>
> - intel_dp->DP = DP;
> -
> if (channel_eq) {
> intel_dp->train_set_valid = true;
> DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n");
> @@ -3904,7 +3897,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
>
> void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> {
> - intel_dp_set_link_train(intel_dp, &intel_dp->DP,
> + intel_dp_set_link_train(intel_dp,
> DP_TRAINING_PATTERN_DISABLE);
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-10-21 14:09 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-05 7:01 [PATCH 00/15] Making DP link training code more readable Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 01/15] drm/i915: Rename DP link training functions Ander Conselvan de Oliveira
2015-10-06 8:54 ` Daniel Vetter
2015-10-05 7:01 ` [PATCH 02/15] drm/i915: Don't pass *DP around to " Ander Conselvan de Oliveira
2015-10-19 4:45 ` Thulasimani, Sivakumar
2015-10-19 7:36 ` Conselvan De Oliveira, Ander
2015-10-19 8:56 ` Ander Conselvan De Oliveira
2015-10-19 9:01 ` Thulasimani, Sivakumar
2015-10-21 13:52 ` [PATCH v2] " Ander Conselvan de Oliveira
2015-10-21 14:08 ` Thulasimani, Sivakumar [this message]
2015-10-05 7:01 ` [PATCH 03/15] drm/i915: Split intel_dp_update_link_train() Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 04/15] drm/i915: Split write of pattern to DP reg from intel_dp_set_link_train Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 05/15] drm/i915: Don't call intel_dp_set_signal_levels() on link train reset Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 06/15] drm/i915: Move generic link training code to a separate file Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 07/15] drm/i915: Create intel_dp->prepare_link_retrain() hook Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 08/15] drm/i915: Make intel_dp_source_supports_hbr2() take an intel_dp pointer Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 09/15] drm/i915: Move link training setup code to separate functions Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 10/15] drm/i915: Move test for max voltage on all lanes to separate function Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 11/15] drm/i915: Add function for getting the current link training voltage Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 12/15] drm/i915: Split full retries loop out of clock recovery code Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 13/15] drm/i915: Make the link training test for same voltage Ander Conselvan de Oliveira
2015-10-06 10:41 ` [PATCH v2] drm/i915: Make the link training test for same voltage smaller Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 14/15] drm/i915: Move the voltage changed check into intel_get_adjust_train() Ander Conselvan de Oliveira
2015-10-05 7:01 ` [PATCH 15/15] drm/i915: Add missing newline to link training debug message Ander Conselvan de Oliveira
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=56279C4E.90301@intel.com \
--to=sivakumar.thulasimani@intel.com \
--cc=ander.conselvan.de.oliveira@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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 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.