Re: [PATCH 2/2] drm/i915/dp: For PCON TMDS mode set only the relavant bits in config DPCD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/3/2021 4:54 PM, Shankar, Uma wrote:

-----Original Message-----
From: Nautiyal, Ankit K <ankit.k.nautiyal@xxxxxxxxx>
Sent: Friday, October 29, 2021 11:32 AM
To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Sharma, Swati2 <swati2.sharma@xxxxxxxxx>; Shankar, Uma
<uma.shankar@xxxxxxxxx>
Subject: [PATCH 2/2] drm/i915/dp: For PCON TMDS mode set only the relavant bits
in config DPCD

Currently we reset the whole PCON linkConfig DPCD to set the TMDS mode.
This also resets the Source control bit and HDMI link enable bit and goes to
autonomous mode of operation, which is seen to spoil the PCONs internal state.

This patch avoids resetting the PCON link config register and sets only TMDS mode
bit, Source control bit in the configuration DPCD.
It then enables the HDMI Link Enable bit.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
---
  drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++++++++-
  1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index f5fd106e555c..3df35079580a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2287,6 +2287,29 @@ static bool intel_dp_is_hdmi_2_1_sink(struct intel_dp
*intel_dp)
  return false;
  }

+static
+int intel_dp_pcon_set_tmds_mode(struct intel_dp *intel_dp) {
+int ret;
+u8 buf = 0;
+
+/* Set PCON source control mode and TMDS */
+buf |= DP_PCON_ENABLE_SOURCE_CTL_MODE;
+buf &= ~DP_PCON_ENABLE_MAX_FRL_BW;
+
I don't see setting this to TMDS, we should be still in FRL mode as bit5 is not being reset here.
Am I missing something ?

Regards,
Uma Shankar

Hi Uma,

Thanks for the reviews.

The buf is initialized to 0, so the bit would be reset.

In fact in code above the reset of MAX_FRL_BW is also redundant as those bits are already 0.

So only thing worth setting here is the SRC_CTL bit followed by HDMI link enable bit.

I would remove the redundant line and resend the patch.

Thanks & Regards,

Ankit


+ret = drm_dp_dpcd_writeb(&intel_dp->aux,
DP_PCON_HDMI_LINK_CONFIG_1, buf);
+if (ret < 0)
+return ret;
+
+/* Set HDMI LINK ENABLE */
+buf |= DP_PCON_ENABLE_HDMI_LINK;
+ret = drm_dp_dpcd_writeb(&intel_dp->aux,
DP_PCON_HDMI_LINK_CONFIG_1, buf);
+if (ret < 0)
+return ret;
+
+return 0;
+}
+
  void intel_dp_check_frl_training(struct intel_dp *intel_dp)  {
  struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -2305,7
+2328,7 @@ void intel_dp_check_frl_training(struct intel_dp *intel_dp)
  int ret, mode;

  drm_dbg(&dev_priv->drm, "Couldn't set FRL mode, continuing with
TMDS mode\n");
-ret = drm_dp_pcon_reset_frl_config(&intel_dp->aux);
+ret = intel_dp_pcon_set_tmds_mode(intel_dp);
  mode = drm_dp_pcon_hdmi_link_mode(&intel_dp->aux, NULL);

  if (ret < 0 || mode != DP_PCON_HDMI_MODE_TMDS)
--
2.25.1



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux