diff mbox series

[05/11] drm/bridge: tc358768: Print logical values, not raw register values

Message ID 20230804-tc358768-v1-5-1afd44b7826b@ideasonboard.com
State New
Headers
Series drm/bridge: tc358768: Fixes and timings improvements |

Commit Message

Tomi Valkeinen Aug. 4, 2023, 10:44 a.m. UTC 
  The driver debug prints DSI related timings as raw register values in
hex. It is much more useful to see the "logical" value of the timing,
not the register value.

Change the prints to print the values separately, in case a single
register contains multiple values, and use %u to have it in a more human
consumable form.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/tc358768.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Péter Ujfalusi Aug. 11, 2023, 4:31 p.m. UTC | #1 
On 04/08/2023 13:44, Tomi Valkeinen wrote:
> The driver debug prints DSI related timings as raw register values in
> hex. It is much more useful to see the "logical" value of the timing,
> not the register value.

I'm a bit confused by the term 'logical' value, I think you meant
decimal, easier to read by humans numbers.

> Change the prints to print the values separately, in case a single
> register contains multiple values, and use %u to have it in a more human
> consumable form.

But, yes, decimal is better for the dmesg, as I recall I had a tool
which was using hex numbers so it was better to have the prints also in hex.

Reviewed-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>

> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/gpu/drm/bridge/tc358768.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
> index 9b633038af33..0ef51d04bb21 100644
> --- a/drivers/gpu/drm/bridge/tc358768.c
> +++ b/drivers/gpu/drm/bridge/tc358768.c
> @@ -739,57 +739,59 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>  
>  	/* LP11 > 100us for D-PHY Rx Init */
>  	val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1;
> -	dev_dbg(priv->dev, "LINEINITCNT: 0x%x\n", val);
> +	dev_dbg(priv->dev, "LINEINITCNT: %u\n", val);
>  	tc358768_write(priv, TC358768_LINEINITCNT, val);
>  
>  	/* LPTimeCnt > 50ns */
>  	val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1;
>  	lptxcnt = val;
> -	dev_dbg(priv->dev, "LPTXTIMECNT: 0x%x\n", val);
> +	dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val);
>  	tc358768_write(priv, TC358768_LPTXTIMECNT, val);
>  
>  	/* 38ns < TCLK_PREPARE < 95ns */
>  	val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1;
> +	dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val);
>  	/* TCLK_PREPARE + TCLK_ZERO > 300ns */
>  	val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk),
>  				  dsibclk_nsk) - 2;
> +	dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2);
>  	val |= val2 << 8;
> -	dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val);
>  	tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);
>  
>  	/* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */
>  	raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5;
>  	val = clamp(raw_val, 0, 127);
> -	dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val);
> +	dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val);
>  	tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);
>  
>  	/* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
>  	val = 50 + tc358768_to_ns(4 * ui_nsk);
>  	val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
> +	dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val);
>  	/* THS_PREPARE + THS_ZERO > 145ns + 10*UI */
>  	raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10;
>  	val2 = clamp(raw_val, 0, 127);
> +	dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2);
>  	val |= val2 << 8;
> -	dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val);
>  	tc358768_write(priv, TC358768_THS_HEADERCNT, val);
>  
>  	/* TWAKEUP > 1ms in lptxcnt steps */
>  	val = tc358768_ns_to_cnt(1020000, dsibclk_nsk);
>  	val = val / (lptxcnt + 1) - 1;
> -	dev_dbg(priv->dev, "TWAKEUP: 0x%x\n", val);
> +	dev_dbg(priv->dev, "TWAKEUP: %u\n", val);
>  	tc358768_write(priv, TC358768_TWAKEUP, val);
>  
>  	/* TCLK_POSTCNT > 60ns + 52*UI */
>  	val = tc358768_ns_to_cnt(60 + tc358768_to_ns(52 * ui_nsk),
>  				 dsibclk_nsk) - 3;
> -	dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val);
> +	dev_dbg(priv->dev, "TCLK_POSTCNT: %u\n", val);
>  	tc358768_write(priv, TC358768_TCLK_POSTCNT, val);
>  
>  	/* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */
>  	raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk),
>  				     dsibclk_nsk) - 4;
>  	val = clamp(raw_val, 0, 15);
> -	dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val);
> +	dev_dbg(priv->dev, "THS_TRAILCNT: %u\n", val);
>  	tc358768_write(priv, TC358768_THS_TRAILCNT, val);
>  
>  	val = BIT(0);
> @@ -803,10 +805,11 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>  	/* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
>  	val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
>  	val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1;
> +	dev_dbg(priv->dev, "TXTAGOCNT: %u\n", val);
>  	val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk),
>  				  dsibclk_nsk) - 2;
> +	dev_dbg(priv->dev, "RXTASURECNT: %u\n", val2);
>  	val = val << 16 | val2;
> -	dev_dbg(priv->dev, "BTACNTRL1: 0x%x\n", val);
>  	tc358768_write(priv, TC358768_BTACNTRL1, val);
>  
>  	/* START[0] */
>
Tomi Valkeinen Aug. 11, 2023, 5:05 p.m. UTC | #2 
On 11/08/2023 19:31, Péter Ujfalusi wrote:
> 
> 
> On 04/08/2023 13:44, Tomi Valkeinen wrote:
>> The driver debug prints DSI related timings as raw register values in
>> hex. It is much more useful to see the "logical" value of the timing,
>> not the register value.
> 
> I'm a bit confused by the term 'logical' value, I think you meant
> decimal, easier to read by humans numbers.

Not just decimal. Previously the code printed the register values, which 
e.g. could contain two values ORed together. So, with "logical" I just 
meant the "real" value, instead of "register-encoded".

>> Change the prints to print the values separately, in case a single
>> register contains multiple values, and use %u to have it in a more human
>> consumable form.
> 
> But, yes, decimal is better for the dmesg, as I recall I had a tool
> which was using hex numbers so it was better to have the prints also in hex.
> 
> Reviewed-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> 
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/bridge/tc358768.c | 21 ++++++++++++---------
>>   1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
>> index 9b633038af33..0ef51d04bb21 100644
>> --- a/drivers/gpu/drm/bridge/tc358768.c
>> +++ b/drivers/gpu/drm/bridge/tc358768.c
>> @@ -739,57 +739,59 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>>   
>>   	/* LP11 > 100us for D-PHY Rx Init */
>>   	val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1;
>> -	dev_dbg(priv->dev, "LINEINITCNT: 0x%x\n", val);
>> +	dev_dbg(priv->dev, "LINEINITCNT: %u\n", val);
>>   	tc358768_write(priv, TC358768_LINEINITCNT, val);
>>   
>>   	/* LPTimeCnt > 50ns */
>>   	val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1;
>>   	lptxcnt = val;
>> -	dev_dbg(priv->dev, "LPTXTIMECNT: 0x%x\n", val);
>> +	dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val);
>>   	tc358768_write(priv, TC358768_LPTXTIMECNT, val);
>>   
>>   	/* 38ns < TCLK_PREPARE < 95ns */
>>   	val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1;
>> +	dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val);
>>   	/* TCLK_PREPARE + TCLK_ZERO > 300ns */
>>   	val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk),
>>   				  dsibclk_nsk) - 2;
>> +	dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2);
>>   	val |= val2 << 8;
>> -	dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val);
>>   	tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);
>>   
>>   	/* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */
>>   	raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5;
>>   	val = clamp(raw_val, 0, 127);
>> -	dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val);
>> +	dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val);
>>   	tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);
>>   
>>   	/* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
>>   	val = 50 + tc358768_to_ns(4 * ui_nsk);
>>   	val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
>> +	dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val);
>>   	/* THS_PREPARE + THS_ZERO > 145ns + 10*UI */
>>   	raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10;
>>   	val2 = clamp(raw_val, 0, 127);
>> +	dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2);
>>   	val |= val2 << 8;
>> -	dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val);
>>   	tc358768_write(priv, TC358768_THS_HEADERCNT, val);
>>   
>>   	/* TWAKEUP > 1ms in lptxcnt steps */
>>   	val = tc358768_ns_to_cnt(1020000, dsibclk_nsk);
>>   	val = val / (lptxcnt + 1) - 1;
>> -	dev_dbg(priv->dev, "TWAKEUP: 0x%x\n", val);
>> +	dev_dbg(priv->dev, "TWAKEUP: %u\n", val);
>>   	tc358768_write(priv, TC358768_TWAKEUP, val);
>>   
>>   	/* TCLK_POSTCNT > 60ns + 52*UI */
>>   	val = tc358768_ns_to_cnt(60 + tc358768_to_ns(52 * ui_nsk),
>>   				 dsibclk_nsk) - 3;
>> -	dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val);
>> +	dev_dbg(priv->dev, "TCLK_POSTCNT: %u\n", val);
>>   	tc358768_write(priv, TC358768_TCLK_POSTCNT, val);
>>   
>>   	/* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */
>>   	raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk),
>>   				     dsibclk_nsk) - 4;
>>   	val = clamp(raw_val, 0, 15);
>> -	dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val);
>> +	dev_dbg(priv->dev, "THS_TRAILCNT: %u\n", val);
>>   	tc358768_write(priv, TC358768_THS_TRAILCNT, val);
>>   
>>   	val = BIT(0);
>> @@ -803,10 +805,11 @@ static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
>>   	/* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
>>   	val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
>>   	val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1;
>> +	dev_dbg(priv->dev, "TXTAGOCNT: %u\n", val);
>>   	val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk),
>>   				  dsibclk_nsk) - 2;
>> +	dev_dbg(priv->dev, "RXTASURECNT: %u\n", val2);
>>   	val = val << 16 | val2;
>> -	dev_dbg(priv->dev, "BTACNTRL1: 0x%x\n", val);
>>   	tc358768_write(priv, TC358768_BTACNTRL1, val);
>>   
>>   	/* START[0] */
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/tc358768.c b/drivers/gpu/drm/bridge/tc358768.c
index 9b633038af33..0ef51d04bb21 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -739,57 +739,59 @@  static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
 
 	/* LP11 > 100us for D-PHY Rx Init */
 	val = tc358768_ns_to_cnt(100 * 1000, dsibclk_nsk) - 1;
-	dev_dbg(priv->dev, "LINEINITCNT: 0x%x\n", val);
+	dev_dbg(priv->dev, "LINEINITCNT: %u\n", val);
 	tc358768_write(priv, TC358768_LINEINITCNT, val);
 
 	/* LPTimeCnt > 50ns */
 	val = tc358768_ns_to_cnt(50, dsibclk_nsk) - 1;
 	lptxcnt = val;
-	dev_dbg(priv->dev, "LPTXTIMECNT: 0x%x\n", val);
+	dev_dbg(priv->dev, "LPTXTIMECNT: %u\n", val);
 	tc358768_write(priv, TC358768_LPTXTIMECNT, val);
 
 	/* 38ns < TCLK_PREPARE < 95ns */
 	val = tc358768_ns_to_cnt(65, dsibclk_nsk) - 1;
+	dev_dbg(priv->dev, "TCLK_PREPARECNT %u\n", val);
 	/* TCLK_PREPARE + TCLK_ZERO > 300ns */
 	val2 = tc358768_ns_to_cnt(300 - tc358768_to_ns(2 * ui_nsk),
 				  dsibclk_nsk) - 2;
+	dev_dbg(priv->dev, "TCLK_ZEROCNT %u\n", val2);
 	val |= val2 << 8;
-	dev_dbg(priv->dev, "TCLK_HEADERCNT: 0x%x\n", val);
 	tc358768_write(priv, TC358768_TCLK_HEADERCNT, val);
 
 	/* TCLK_TRAIL > 60ns AND TEOT <= 105 ns + 12*UI */
 	raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(2 * ui_nsk), dsibclk_nsk) - 5;
 	val = clamp(raw_val, 0, 127);
-	dev_dbg(priv->dev, "TCLK_TRAILCNT: 0x%x\n", val);
+	dev_dbg(priv->dev, "TCLK_TRAILCNT: %u\n", val);
 	tc358768_write(priv, TC358768_TCLK_TRAILCNT, val);
 
 	/* 40ns + 4*UI < THS_PREPARE < 85ns + 6*UI */
 	val = 50 + tc358768_to_ns(4 * ui_nsk);
 	val = tc358768_ns_to_cnt(val, dsibclk_nsk) - 1;
+	dev_dbg(priv->dev, "THS_PREPARECNT %u\n", val);
 	/* THS_PREPARE + THS_ZERO > 145ns + 10*UI */
 	raw_val = tc358768_ns_to_cnt(145 - tc358768_to_ns(3 * ui_nsk), dsibclk_nsk) - 10;
 	val2 = clamp(raw_val, 0, 127);
+	dev_dbg(priv->dev, "THS_ZEROCNT %u\n", val2);
 	val |= val2 << 8;
-	dev_dbg(priv->dev, "THS_HEADERCNT: 0x%x\n", val);
 	tc358768_write(priv, TC358768_THS_HEADERCNT, val);
 
 	/* TWAKEUP > 1ms in lptxcnt steps */
 	val = tc358768_ns_to_cnt(1020000, dsibclk_nsk);
 	val = val / (lptxcnt + 1) - 1;
-	dev_dbg(priv->dev, "TWAKEUP: 0x%x\n", val);
+	dev_dbg(priv->dev, "TWAKEUP: %u\n", val);
 	tc358768_write(priv, TC358768_TWAKEUP, val);
 
 	/* TCLK_POSTCNT > 60ns + 52*UI */
 	val = tc358768_ns_to_cnt(60 + tc358768_to_ns(52 * ui_nsk),
 				 dsibclk_nsk) - 3;
-	dev_dbg(priv->dev, "TCLK_POSTCNT: 0x%x\n", val);
+	dev_dbg(priv->dev, "TCLK_POSTCNT: %u\n", val);
 	tc358768_write(priv, TC358768_TCLK_POSTCNT, val);
 
 	/* max(60ns + 4*UI, 8*UI) < THS_TRAILCNT < 105ns + 12*UI */
 	raw_val = tc358768_ns_to_cnt(60 + tc358768_to_ns(18 * ui_nsk),
 				     dsibclk_nsk) - 4;
 	val = clamp(raw_val, 0, 15);
-	dev_dbg(priv->dev, "THS_TRAILCNT: 0x%x\n", val);
+	dev_dbg(priv->dev, "THS_TRAILCNT: %u\n", val);
 	tc358768_write(priv, TC358768_THS_TRAILCNT, val);
 
 	val = BIT(0);
@@ -803,10 +805,11 @@  static void tc358768_bridge_pre_enable(struct drm_bridge *bridge)
 	/* TXTAGOCNT[26:16] RXTASURECNT[10:0] */
 	val = tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk * 4);
 	val = tc358768_ns_to_cnt(val, dsibclk_nsk) / 4 - 1;
+	dev_dbg(priv->dev, "TXTAGOCNT: %u\n", val);
 	val2 = tc358768_ns_to_cnt(tc358768_to_ns((lptxcnt + 1) * dsibclk_nsk),
 				  dsibclk_nsk) - 2;
+	dev_dbg(priv->dev, "RXTASURECNT: %u\n", val2);
 	val = val << 16 | val2;
-	dev_dbg(priv->dev, "BTACNTRL1: 0x%x\n", val);
 	tc358768_write(priv, TC358768_BTACNTRL1, val);
 
 	/* START[0] */