[PATCH v7 09/15] drm/i915: Add support for starting FRL training for HDMI2.1 via PCON
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Tue Feb 2 06:39:47 UTC 2021
Hi Ville,
Please find my responses inline.
On 2/2/2021 2:08 AM, Ville Syrjälä wrote:
> On Fri, Dec 18, 2020 at 04:07:17PM +0530, Ankit Nautiyal wrote:
>> This patch adds functions to start FRL training for an HDMI2.1 sink,
>> connected via a PCON as a DP branch device.
>> This patch also adds a new structure for storing frl training related
>> data, when FRL training is completed.
>>
>> v2: As suggested by Uma Shankar:
>> -renamed couple of variables for better clarity
>> -tweaked the macros used for correct semantics for true/false
>> -fixed other styling issues.
>>
>> v3: Completed the TODO for condition for going to FRL mode.
>> Modified the condition to determine the required FRL b/w
>> based only on the Pcon and Sink's max FRL values.
>> Moved the frl structure initialization to intel_dp_init_connector().
>>
>> v4: Fixed typo in initialization of frl structure.
>>
>> v5: Always use FRL if its possible, instead of enabling only for
>> higher modes as done in v3.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> Reviewed-by: Uma Shankar <uma.shankar at intel.com> (v2)
>> ---
>> .../drm/i915/display/intel_display_types.h | 7 +
>> drivers/gpu/drm/i915/display/intel_dp.c | 151 ++++++++++++++++++
>> drivers/gpu/drm/i915/display/intel_dp.h | 2 +
>> 3 files changed, 160 insertions(+)
> <snip>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 0596d6c24e73..43027a6d5e5e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -3891,6 +3891,8 @@ static void intel_disable_dp(struct intel_atomic_state *state,
>> intel_edp_backlight_off(old_conn_state);
>> intel_dp_set_power(intel_dp, DP_SET_POWER_D3);
>> intel_edp_panel_off(intel_dp);
>> + intel_dp->frl.is_trained = false;
>> + intel_dp->frl.trained_rate_gbps = 0;
> This stuff looks rather misplaced (or missing from elsewhere). This code
> doesn't even get executed on modern platforms.
I think these two lines should have been added to
intel_ddi_post_disable_dp() for TGL+
My intention was to reset these before disabling DP. In hindsight, since
we are initializing (resetting) these in dp_init_connector, this doesnt
seem to be required.
I will send a patch to remove these two lines from here.
>
> <snip>
>> +static int intel_dp_pcon_start_frl_training(struct intel_dp *intel_dp)
>> +{
>> +#define PCON_EXTENDED_TRAIN_MODE (1 > 0)
>> +#define PCON_CONCURRENT_MODE (1 > 0)
>> +#define PCON_SEQUENTIAL_MODE !PCON_CONCURRENT_MODE
>> +#define PCON_NORMAL_TRAIN_MODE !PCON_EXTENDED_TRAIN_MODE
> All of that looks like nonsense. What is it supposed to do?
When asking an HDMI2.1 PCON to initiate FRL training there are 2 options:
Sequential/Concurrent mode: Sequential mode attempts the FRL training
after DP Link training is completed. Concurrent mode tries to do the FRL
training, during DP link training.
Normal train Mode/ Extended mode: Normal train mode, PCON FW trains FRL
from Max to min BW, set by source in BW Mask. It aborts on first
successful training. In Extended mode, PCON FW trains for all BW set by
source in BW mask.
For Concurrent and Extended mode we need to set some extra bits in PCON
FRL config DPCDs
The intention was to go with sequential and Normal training mode, so no
need to set above bits.
Do you think, some documentation will make this clear?
Thanks & Regards,
Ankit
>
>> +#define TIMEOUT_FRL_READY_MS 500
>> +#define TIMEOUT_HDMI_LINK_ACTIVE_MS 1000
>> +
>> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> + int max_frl_bw, max_pcon_frl_bw, max_edid_frl_bw, ret;
>> + u8 max_frl_bw_mask = 0, frl_trained_mask;
>> + bool is_active;
>> +
>> + ret = drm_dp_pcon_reset_frl_config(&intel_dp->aux);
>> + if (ret < 0)
>> + return ret;
>> +
>> + max_pcon_frl_bw = intel_dp->dfp.pcon_max_frl_bw;
>> + drm_dbg(&i915->drm, "PCON max rate = %d Gbps\n", max_pcon_frl_bw);
>> +
>> + max_edid_frl_bw = intel_dp_hdmi_sink_max_frl(intel_dp);
>> + drm_dbg(&i915->drm, "Sink max rate from EDID = %d Gbps\n", max_edid_frl_bw);
>> +
>> + max_frl_bw = min(max_edid_frl_bw, max_pcon_frl_bw);
>> +
>> + if (max_frl_bw <= 0)
>> + return -EINVAL;
>> +
>> + ret = drm_dp_pcon_frl_prepare(&intel_dp->aux, false);
>> + if (ret < 0)
>> + return ret;
>> + /* Wait for PCON to be FRL Ready */
>> + wait_for(is_active = drm_dp_pcon_is_frl_ready(&intel_dp->aux) == true, TIMEOUT_FRL_READY_MS);
>> +
>> + if (!is_active)
>> + return -ETIMEDOUT;
>> +
>> + max_frl_bw_mask = intel_dp_pcon_set_frl_mask(max_frl_bw);
>> + ret = drm_dp_pcon_frl_configure_1(&intel_dp->aux, max_frl_bw, PCON_SEQUENTIAL_MODE);
>> + if (ret < 0)
>> + return ret;
>> + ret = drm_dp_pcon_frl_configure_2(&intel_dp->aux, max_frl_bw_mask, PCON_NORMAL_TRAIN_MODE);
>> + if (ret < 0)
>> + return ret;
>> + ret = drm_dp_pcon_frl_enable(&intel_dp->aux);
>> + if (ret < 0)
>> + return ret;
>> + /*
>> + * Wait for FRL to be completed
>> + * Check if the HDMI Link is up and active.
>> + */
>> + wait_for(is_active = drm_dp_pcon_hdmi_link_active(&intel_dp->aux) == true, TIMEOUT_HDMI_LINK_ACTIVE_MS);
>> +
>> + if (!is_active)
>> + return -ETIMEDOUT;
>> +
>> + /* Verify HDMI Link configuration shows FRL Mode */
>> + if (DP_PCON_HDMI_MODE_FRL != drm_dp_pcon_hdmi_link_mode(&intel_dp->aux, &frl_trained_mask)) {
>> + drm_dbg(&i915->drm, "HDMI couldn't be trained in FRL Mode\n");
>> + return -EINVAL;
>> + }
>> + drm_dbg(&i915->drm, "MAX_FRL_MASK = %u, FRL_TRAINED_MASK = %u\n", max_frl_bw_mask, frl_trained_mask);
>> +
>> + intel_dp->frl.trained_rate_gbps = intel_dp_pcon_get_frl_mask(frl_trained_mask);
>> + intel_dp->frl.is_trained = true;
>> + drm_dbg(&i915->drm, "FRL trained with : %d Gbps\n", intel_dp->frl.trained_rate_gbps);
>> +
>> + return 0;
>> +}
>> +
>> +static bool intel_dp_is_hdmi_2_1_sink(struct intel_dp *intel_dp)
>> +{
>> + if (drm_dp_is_branch(intel_dp->dpcd) &&
>> + intel_dp->has_hdmi_sink &&
>> + intel_dp_hdmi_sink_max_frl(intel_dp) > 0)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +void intel_dp_check_frl_training(struct intel_dp *intel_dp)
>> +{
>> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>> +
>> + /* Always go for FRL training if supported */
>> + if (!intel_dp_is_hdmi_2_1_sink(intel_dp) ||
>> + intel_dp->frl.is_trained)
>> + return;
>> +
>> + if (intel_dp_pcon_start_frl_training(intel_dp) < 0) {
>> + int ret, mode;
>> +
>> + drm_dbg(&dev_priv->drm, "Couldnt set FRL mode, continuing with TMDS mode\n");
>> + ret = drm_dp_pcon_reset_frl_config(&intel_dp->aux);
>> + mode = drm_dp_pcon_hdmi_link_mode(&intel_dp->aux, NULL);
>> +
>> + if (ret < 0 || mode != DP_PCON_HDMI_MODE_TMDS)
>> + drm_dbg(&dev_priv->drm, "Issue with PCON, cannot set TMDS mode\n");
>> + } else {
>> + drm_dbg(&dev_priv->drm, "FRL training Completed\n");
>> + }
>> +}
>> +
>> static void
>> g4x_set_link_train(struct intel_dp *intel_dp,
>> const struct intel_crtc_state *crtc_state,
>> @@ -8210,6 +8358,9 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
>> (temp & ~0xf) | 0xd);
>> }
>>
>> + intel_dp->frl.is_trained = false;
>> + intel_dp->frl.trained_rate_gbps = 0;
>> +
>> return true;
>>
>> fail:
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>> index b871a09b6901..58b76a20459c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -144,4 +144,6 @@ bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
>> void intel_dp_sync_state(struct intel_encoder *encoder,
>> const struct intel_crtc_state *crtc_state);
>>
>> +void intel_dp_check_frl_training(struct intel_dp *intel_dp);
>> +
>> #endif /* __INTEL_DP_H__ */
>> --
>> 2.17.1
More information about the dri-devel
mailing list