From: "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Ander Conselvan de Oliveira"
<ander.conselvan.de.oliveira@intel.com>
Cc: jani.nikula@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 7/8] drm/i915: Move max voltage and pre emphasis to intel_dp_link_training.c
Date: Mon, 19 Oct 2015 10:44:43 +0530 [thread overview]
Message-ID: <56247C43.8040307@intel.com> (raw)
In-Reply-To: <20150908130147.GE29811@intel.com>
As an FYI, both of these functions need to be rewritten when we want the
code to be
compliant to DP spec.
We should read the pre-emphasis given by the panel and if vwing exeeds
max value
we should use the max vswing supported for that pre-emphasis but current
code
is opposite of that. you can read more detailed logic in section
"3.5.1.2 Link Training"
in DP Spec.
by the way same is true for much of the link training logic and related
functions
i don't know if such rewriting should be done after fixing those or
before :(.
regards,
Sivakumar.
On 9/8/2015 6:31 PM, Ville Syrjälä wrote:
> On Tue, Sep 08, 2015 at 03:27:58PM +0300, Ander Conselvan de Oliveira wrote:
>> So link training tests can use real hardware limits.
> These need to be kept in sync with the _signal_levels() functions, so
> moving them to a separate file is a bit questionable.
>
> I suggest that we should attempt to restructure this information as
> some kind of tables, from which we can look up the max values as
> well as the hardware specific values for each setting.
>
> That of course won't solve your problem. So far I don't know what your
> test even does and why does it need this information, so I guess I'll
> need to read on...
>
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 99 ---------------------------
>> drivers/gpu/drm/i915/intel_dp_link_training.c | 92 +++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_drv.h | 11 +--
>> 3 files changed, 99 insertions(+), 103 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 5778059..da87aef 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -111,13 +111,6 @@ static bool is_edp(struct intel_dp *intel_dp)
>> return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
>> }
>>
>> -static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>> -{
>> - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> -
>> - return intel_dig_port->base.base.dev;
>> -}
>> -
>> static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>> {
>> return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
>> @@ -3054,98 +3047,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
>> DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
>> }
>>
>> -/* These are source-specific values. */
>> -uint8_t
>> -intel_dp_voltage_max(struct intel_dp *intel_dp)
>> -{
>> - struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> - struct drm_i915_private *dev_priv = dev->dev_private;
>> - enum port port = dp_to_dig_port(intel_dp)->port;
>> -
>> - if (IS_BROXTON(dev))
>> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> - else if (INTEL_INFO(dev)->gen >= 9) {
>> - if (dev_priv->edp_low_vswing && port == PORT_A)
>> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> - } else if (IS_VALLEYVIEW(dev))
>> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> - else if (IS_GEN7(dev) && port == PORT_A)
>> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> - else if (HAS_PCH_CPT(dev) && port != PORT_A)
>> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> - else
>> - return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> -}
>> -
>> -uint8_t
>> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
>> -{
>> - struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> - enum port port = dp_to_dig_port(intel_dp)->port;
>> -
>> - if (INTEL_INFO(dev)->gen >= 9) {
>> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> - default:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> - }
>> - } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> - default:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> - }
>> - } else if (IS_VALLEYVIEW(dev)) {
>> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> - default:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> - }
>> - } else if (IS_GEN7(dev) && port == PORT_A) {
>> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> - default:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> - }
>> - } else {
>> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> - default:
>> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> - }
>> - }
>> -}
>> -
>> static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
>> {
>> struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> index f33cbbb..8d27dce 100644
>> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> @@ -23,6 +23,98 @@
>>
>> #include "intel_drv.h"
>>
>> +/* These are source-specific values. */
>> +static uint8_t
>> +intel_dp_voltage_max(struct intel_dp *intel_dp)
>> +{
>> + struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + enum port port = dp_to_dig_port(intel_dp)->port;
>> +
>> + if (IS_BROXTON(dev))
>> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> + else if (INTEL_INFO(dev)->gen >= 9) {
>> + if (dev_priv->edp_low_vswing && port == PORT_A)
>> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> + } else if (IS_VALLEYVIEW(dev))
>> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> + else if (IS_GEN7(dev) && port == PORT_A)
>> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> + else if (HAS_PCH_CPT(dev) && port != PORT_A)
>> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
>> + else
>> + return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>> +}
>> +
>> +static uint8_t
>> +intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
>> +{
>> + struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> + enum port port = dp_to_dig_port(intel_dp)->port;
>> +
>> + if (INTEL_INFO(dev)->gen >= 9) {
>> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> + default:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> + }
>> + } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> + default:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> + }
>> + } else if (IS_VALLEYVIEW(dev)) {
>> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> + default:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> + }
>> + } else if (IS_GEN7(dev) && port == PORT_A) {
>> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> + default:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> + }
>> + } else {
>> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
>> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>> + default:
>> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
>> + }
>> + }
>> +}
>> +
>> static void
>> intel_get_adjust_train(struct intel_dp *intel_dp,
>> const uint8_t link_status[DP_LINK_STATUS_SIZE])
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 29ae4bb..671a20f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1216,15 +1216,18 @@ intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
>> void
>> intel_dp_update_signal_levels(struct intel_dp *intel_dp);
>> void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
>> -uint8_t
>> -intel_dp_voltage_max(struct intel_dp *intel_dp);
>> -uint8_t
>> -intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing);
>> void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
>> uint8_t *link_bw, uint8_t *rate_select);
>> bool intel_dp_source_supports_hbr2(struct drm_device *dev);
>> bool
>> intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]);
>> +static inline struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>> +{
>> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +
>> + return intel_dig_port->base.base.dev;
>> +}
>> +
>>
>> /* intel_dp_mst.c */
>> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
>> --
>> 2.4.3
_______________________________________________
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-19 5:14 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-08 12:27 [PATCH 0/8] [RFC] Link training test Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 1/8] drm/i915: Rename DP link training functions Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 2/8] drm/i915: Don't pass *DP around to " Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 3/8] drm/i915: Split intel_dp_update_link_train() Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 4/8] drm/i915: Split write of pattern to DP reg from intel_dp_set_link_train Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 5/8] drm/i915: Don't call intel_dp_set_signal_levels() on link train reset Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 6/8] drm/i915: Move generic link training code to a separate file Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 7/8] drm/i915: Move max voltage and pre emphasis to intel_dp_link_training.c Ander Conselvan de Oliveira
2015-09-08 13:01 ` Ville Syrjälä
2015-10-19 5:14 ` Thulasimani, Sivakumar [this message]
2015-10-19 5:59 ` Ander Conselvan De Oliveira
2015-10-19 6:13 ` Thulasimani, Sivakumar
2015-09-08 12:27 ` [PATCH 8/8] drm/i915: Extract intel_dev_info.[ch] Ander Conselvan de Oliveira
2015-09-08 12:28 ` [PATCH i-g-t] Add a link training test Ander Conselvan de Oliveira
2015-09-08 13:11 ` Ville Syrjälä
2015-09-08 13:26 ` Ander Conselvan De Oliveira
2015-09-09 10:33 ` Thomas Wood
2015-09-11 14:11 ` [PATCH] Add a link training test (v2) Ander Conselvan de Oliveira
2015-09-14 11:51 ` [PATCH] drm/i915: Add link training test Ander Conselvan de Oliveira
2015-09-14 13:11 ` Daniel Vetter
2015-09-14 13:38 ` Ander Conselvan De Oliveira
2015-09-15 13:08 ` Ander Conselvan De Oliveira
2015-09-23 8:24 ` Daniel Vetter
2015-09-23 9:18 ` Ander Conselvan De Oliveira
2015-09-23 9:38 ` Shared scaffolding to unit-test kernel code in userspace (was Re: [Intel-gfx] [PATCH] drm/i915: Add link training test) Daniel Vetter
2015-09-23 9:38 ` Daniel Vetter
2015-09-29 8:47 ` Ander Conselvan De Oliveira
2015-09-29 8:47 ` Shared scaffolding to unit-test kernel code in userspace (was " Ander Conselvan De Oliveira
2015-09-15 13:11 ` [PATCH 1/2] drm/i915: Make link training state machine code use function pointers Ander Conselvan de Oliveira
2015-09-15 13:11 ` [PATCH 2/2] drm/i915: Add a link training test Ander Conselvan de Oliveira
2015-09-22 15:12 ` [PATCH 0/8] [RFC] Link " Daniel Vetter
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=56247C43.8040307@intel.com \
--to=sivakumar.thulasimani@intel.com \
--cc=ander.conselvan.de.oliveira@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=ville.syrjala@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 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.