From: "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com>
To: Ander Conselvan de Oliveira
<ander.conselvan.de.oliveira@intel.com>,
intel-gfx@lists.freedesktop.org, jim.bride@linux.intel.com
Subject: Re: [PATCH 01/22] drm/i915: Don't pass *DP around to link training functions
Date: Sun, 25 Oct 2015 07:31:40 +0530 [thread overview]
Message-ID: <562C3804.30708@intel.com> (raw)
In-Reply-To: <1445594525-7174-2-git-send-email-ander.conselvan.de.oliveira@intel.com>
Thanks for making the changes suggested :)
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
On 10/23/2015 3:31 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)
>
> v3: Remove DP parameter from intel_dp_set_signal_levels(). (Sivakumar)
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 47 ++++++++++++++++++-----------------------
> 1 file changed, 20 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8287df4..648300e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3552,7 +3552,7 @@ gen7_edp_signal_levels(uint8_t train_set)
>
> /* Properly updates "DP" with the correct signal levels. */
> static void
> -intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
> +intel_dp_set_signal_levels(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> enum port port = intel_dig_port->port;
> @@ -3591,12 +3591,11 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
> (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) >>
> DP_TRAIN_PRE_EMPHASIS_SHIFT);
>
> - *DP = (*DP & ~mask) | signal_levels;
> + intel_dp->DP = (intel_dp->DP & ~mask) | signal_levels;
> }
>
> 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);
> + 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);
>
> - 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-25 2:01 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-23 10:01 [PATCH 00/22] DP refactoring v2 Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 01/22] drm/i915: Don't pass *DP around to link training functions Ander Conselvan de Oliveira
2015-10-25 2:01 ` Thulasimani, Sivakumar [this message]
2015-10-23 10:01 ` [PATCH 02/22] drm/i915: Split write of pattern to DP reg from intel_dp_set_link_train Ander Conselvan de Oliveira
2015-10-25 2:06 ` Thulasimani, Sivakumar
2015-10-23 10:01 ` [PATCH 03/22] drm/i915 Call get_adjust_train() from clock recovery and channel eq Ander Conselvan de Oliveira
2015-10-25 2:11 ` Thulasimani, Sivakumar
2015-10-23 10:01 ` [PATCH 04/22] drm/i915: Move register write into intel_dp_set_signal_levels() Ander Conselvan de Oliveira
2015-10-25 2:28 ` Thulasimani, Sivakumar
2015-10-23 10:01 ` [PATCH 05/22] drm/i915: Move generic link training code to a separate file Ander Conselvan de Oliveira
2015-10-25 2:37 ` Thulasimani, Sivakumar
2015-10-23 10:01 ` [PATCH 06/22] drm/i915: Create intel_dp->prepare_link_retrain() hook Ander Conselvan de Oliveira
2015-10-25 2:40 ` Thulasimani, Sivakumar
2015-10-23 10:01 ` [PATCH 07/22] drm/i915: Make intel_dp_source_supports_hbr2() take an intel_dp pointer Ander Conselvan de Oliveira
2015-10-25 2:48 ` Thulasimani, Sivakumar
2015-11-05 13:22 ` Ander Conselvan De Oliveira
2015-10-23 10:01 ` [PATCH 08/22] drm/i915: Move link training setup code to separate functions (v2) Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 09/22] drm/i915: Move test for max voltage on all lanes to separate function Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 10/22] drm/i915: Add function for getting the current link training voltage Ander Conselvan de Oliveira
2015-10-25 3:56 ` Thulasimani, Sivakumar
2015-10-23 10:01 ` [PATCH 11/22] drm/i915: Split full retries loop out of clock recovery code (v2) Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 12/22] drm/i915: Make the link training test for same voltage smaller Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 13/22] drm/i915: Move the voltage changed check into intel_get_adjust_train() Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 14/22] drm/i915: Add missing newline to link training debug message Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 15/22] drm/i915: Split setting of vswing and pre_emph levels to separate file Ander Conselvan de Oliveira
2015-10-23 10:01 ` [PATCH 16/22] drm/i915: Introduce struct intel_dp_signal_levels Ander Conselvan de Oliveira
2015-10-25 5:24 ` Thulasimani, Sivakumar
2015-10-26 7:22 ` Ander Conselvan De Oliveira
2015-10-23 10:02 ` [PATCH 17/22] drm/i915: Use struct intel_dp_signal_levels for eDP on SNB and IVB Ander Conselvan de Oliveira
2015-10-23 10:02 ` [PATCH 18/22] drm/i915: Use struct intel_dp_signal_levels for VLV Ander Conselvan de Oliveira
2015-10-23 10:02 ` [PATCH 19/22] drm/i915: Use struct intel_dp_signal_levels for CHV Ander Conselvan de Oliveira
2015-10-23 10:02 ` [PATCH 20/22] drm/i915: Use struct intel_dp_signal_levels for DDI platforms Ander Conselvan de Oliveira
2015-10-23 10:02 ` [PATCH 21/22] drm/i915: Remove old functions for maximum DP vswing and pre-emph levels Ander Conselvan de Oliveira
2015-10-23 10:02 ` [PATCH 22/22] drm/i915: Move ddi_signal_levels() to intel_dp_signal_levels.c 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=562C3804.30708@intel.com \
--to=sivakumar.thulasimani@intel.com \
--cc=ander.conselvan.de.oliveira@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jim.bride@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