Re: [Intel-gfx] [PATCH] drm/scdc-helper: Pimp SCDC debugs

2023-03-30 Thread Ville Syrjälä
On Thu, Mar 30, 2023 at 10:44:04AM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Wed, Mar 29, 2023 at 08:14:02PM +0300, Ville Syrjala wrote:
> > diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c 
> > b/drivers/gpu/drm/display/drm_scdc_helper.c
> > index c3ad4ab2b456..2b124152384c 100644
> > --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> > +++ b/drivers/gpu/drm/display/drm_scdc_helper.c
> > @@ -26,6 +26,8 @@
> >  #include 
> >  
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> >  
> >  /**
> > @@ -140,6 +142,7 @@ EXPORT_SYMBOL(drm_scdc_write);
> >  
> >  /**
> >   * drm_scdc_get_scrambling_status - what is status of scrambling?
> > + * @connector: connector
> >   * @adapter: I2C adapter for DDC channel
> >   *
> >   * Reads the scrambler status over SCDC, and checks the
> > @@ -148,14 +151,17 @@ EXPORT_SYMBOL(drm_scdc_write);
> >   * Returns:
> >   * True if the scrambling is enabled, false otherwise.
> >   */
> > -bool drm_scdc_get_scrambling_status(struct i2c_adapter *adapter)
> > +bool drm_scdc_get_scrambling_status(struct drm_connector *connector,
> > +   struct i2c_adapter *adapter)
> 
> Is there any driver where adapter isn't equal to connector->ddc?

I figured most of them since they all maintain their own ddc
pointer elsewhere. But looks like the drivers using the scdc
helper might all be setting connector->ddc for their HDMI
connectors despite that.

Even i915 does that apparently. My recollection was otherwise
since I have a branch waiting somewhere that changes the whole
driver to use connector->ddc.

I guess someone took a bunch of shortcuts to get connector->ddc
populated but then didn't finish the job by clearing out the
old pointers.

> 
> If not, there's no reason to pass both

Yeah, that did cross my mind. But left it out with in
the belief that it can't be done yet.

-- 
Ville Syrjälä
Intel


Re: [Intel-gfx] [PATCH] drm/scdc-helper: Pimp SCDC debugs

2023-03-30 Thread Maxime Ripard
Hi,

On Wed, Mar 29, 2023 at 08:14:02PM +0300, Ville Syrjala wrote:
> diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c 
> b/drivers/gpu/drm/display/drm_scdc_helper.c
> index c3ad4ab2b456..2b124152384c 100644
> --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> +++ b/drivers/gpu/drm/display/drm_scdc_helper.c
> @@ -26,6 +26,8 @@
>  #include 
>  
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  /**
> @@ -140,6 +142,7 @@ EXPORT_SYMBOL(drm_scdc_write);
>  
>  /**
>   * drm_scdc_get_scrambling_status - what is status of scrambling?
> + * @connector: connector
>   * @adapter: I2C adapter for DDC channel
>   *
>   * Reads the scrambler status over SCDC, and checks the
> @@ -148,14 +151,17 @@ EXPORT_SYMBOL(drm_scdc_write);
>   * Returns:
>   * True if the scrambling is enabled, false otherwise.
>   */
> -bool drm_scdc_get_scrambling_status(struct i2c_adapter *adapter)
> +bool drm_scdc_get_scrambling_status(struct drm_connector *connector,
> + struct i2c_adapter *adapter)

Is there any driver where adapter isn't equal to connector->ddc?

If not, there's no reason to pass both

Maxime


[Intel-gfx] [PATCH] drm/scdc-helper: Pimp SCDC debugs

2023-03-29 Thread Ville Syrjala
From: Ville Syrjälä 

Include the device and connector information in the SCDC
debugs. Makes it easier to figure out who did what.

Cc: Andrzej Hajda 
Cc: Neil Armstrong 
Cc: Robert Foss 
Cc: Laurent Pinchart 
Cc: Jonas Karlman 
Cc: Jernej Skrabec 
Cc: Thierry Reding 
Cc: Emma Anholt 
Cc: Maxime Ripard 
Cc: intel-gfx@lists.freedesktop.org
Cc: linux-te...@vger.kernel.org
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  8 ++---
 drivers/gpu/drm/display/drm_scdc_helper.c | 36 ++-
 drivers/gpu/drm/i915/display/intel_ddi.c  |  4 +--
 drivers/gpu/drm/i915/display/intel_hdmi.c |  4 +--
 drivers/gpu/drm/tegra/sor.c   | 10 +++
 drivers/gpu/drm/vc4/vc4_hdmi.c| 21 +++--
 include/drm/display/drm_scdc_helper.h | 12 ++--
 7 files changed, 62 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index aa51c61a78c7..7f57ca168ab6 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1426,9 +1426,9 @@ void dw_hdmi_set_high_tmds_clock_ratio(struct dw_hdmi 
*hdmi,
/* Control for TMDS Bit Period/TMDS Clock-Period Ratio */
if (dw_hdmi_support_scdc(hdmi, display)) {
if (mtmdsclock > HDMI14_MAX_TMDSCLK)
-   drm_scdc_set_high_tmds_clock_ratio(hdmi->ddc, 1);
+   drm_scdc_set_high_tmds_clock_ratio(>connector, 
hdmi->ddc, 1);
else
-   drm_scdc_set_high_tmds_clock_ratio(hdmi->ddc, 0);
+   drm_scdc_set_high_tmds_clock_ratio(>connector, 
hdmi->ddc, 0);
}
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_high_tmds_clock_ratio);
@@ -2116,7 +2116,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
min_t(u8, bytes, SCDC_MIN_SOURCE_VERSION));
 
/* Enabled Scrambling in the Sink */
-   drm_scdc_set_scrambling(hdmi->ddc, 1);
+   drm_scdc_set_scrambling(>connector, hdmi->ddc, 1);
 
/*
 * To activate the scrambler feature, you must ensure
@@ -2132,7 +2132,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
hdmi_writeb(hdmi, 0, HDMI_FC_SCRAMBLER_CTRL);
hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ,
HDMI_MC_SWRSTZ);
-   drm_scdc_set_scrambling(hdmi->ddc, 0);
+   drm_scdc_set_scrambling(>connector, hdmi->ddc, 0);
}
}
 
diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c 
b/drivers/gpu/drm/display/drm_scdc_helper.c
index c3ad4ab2b456..2b124152384c 100644
--- a/drivers/gpu/drm/display/drm_scdc_helper.c
+++ b/drivers/gpu/drm/display/drm_scdc_helper.c
@@ -26,6 +26,8 @@
 #include 
 
 #include 
+#include 
+#include 
 #include 
 
 /**
@@ -140,6 +142,7 @@ EXPORT_SYMBOL(drm_scdc_write);
 
 /**
  * drm_scdc_get_scrambling_status - what is status of scrambling?
+ * @connector: connector
  * @adapter: I2C adapter for DDC channel
  *
  * Reads the scrambler status over SCDC, and checks the
@@ -148,14 +151,17 @@ EXPORT_SYMBOL(drm_scdc_write);
  * Returns:
  * True if the scrambling is enabled, false otherwise.
  */
-bool drm_scdc_get_scrambling_status(struct i2c_adapter *adapter)
+bool drm_scdc_get_scrambling_status(struct drm_connector *connector,
+   struct i2c_adapter *adapter)
 {
u8 status;
int ret;
 
ret = drm_scdc_readb(adapter, SCDC_SCRAMBLER_STATUS, );
if (ret < 0) {
-   DRM_DEBUG_KMS("Failed to read scrambling status: %d\n", ret);
+   drm_dbg_kms(connector->dev,
+   "[CONNECTOR:%d:%s] Failed to read scrambling 
status: %d\n",
+   connector->base.id, connector->name, ret);
return false;
}
 
@@ -165,6 +171,7 @@ EXPORT_SYMBOL(drm_scdc_get_scrambling_status);
 
 /**
  * drm_scdc_set_scrambling - enable scrambling
+ * @connector: connector
  * @adapter: I2C adapter for DDC channel
  * @enable: bool to indicate if scrambling is to be enabled/disabled
  *
@@ -175,14 +182,18 @@ EXPORT_SYMBOL(drm_scdc_get_scrambling_status);
  * Returns:
  * True if scrambling is set/reset successfully, false otherwise.
  */
-bool drm_scdc_set_scrambling(struct i2c_adapter *adapter, bool enable)
+bool drm_scdc_set_scrambling(struct drm_connector *connector,
+struct i2c_adapter *adapter,
+bool enable)
 {
u8 config;
int ret;
 
ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, );
if (ret < 0) {
-   DRM_DEBUG_KMS("Failed to read TMDS config: %d\n", ret);
+   drm_dbg_kms(connector->dev,
+   "[CONNECTOR:%d:%s] Failed to read TMDS config: 
%d\n",
+