Re: [PATCH 2/2] drm/i915/dsi: fix dphy param field widths and range checks for chv+

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

 



On Thu, 19 Apr 2018, Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
> On Thu, 19 Apr 2018, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> On Thu, Apr 19, 2018 at 11:59:40AM +0300, Jani Nikula wrote:
>>> Current CHV and BXT bspec says the dphy param register has four 8-bit
>>> fields instead of the smaller VLV field widths. CHV bspec mentions the
>>> register has changed since K0, but there's no indication of what exactly
>>> changed. Lacking further details, change the field widths for all CHV
>>> and later.
>>
>> K0 didn't happen, and looks like the linked HSD specifically says that
>> this change was meant for K0. So I suspect this patch is wrong.
>
> Oh well. The referenced bug indicated overflow in the logs, so I figured
> this might be it.
>
> I pushed patch 1 for now.
>
>> The BXT HSD says planned for B0, but not sure if that actually happened
>> either.
>>
>> On my VLV the reserved bits can't be set:
>> intel_reg write 0x180000:0xb080 0xffffffff
>> intel_reg write 0x180000:0xb880 0xffffffff
>> intel_reg read 0x180000:0xb080 0x180000:0xb880
>>  (0x00180000:0x0000b080): 0x3f1fff3f
>>  (0x00180000:0x0000b880): 0x3f1fff3f
>>
>> So presumably the same rmw trick could be used to check what
>> how many bits we have on CHV/BXT.
>
> Asked on the bug, let's see if we get a response.

Confirms what you said:

(0x00180000:0x0000b080): 0x3f1fff3f
(0x00180000:0x0000b880): 0x3f1fff3f

BR,
Jani.

>
> BR,
> Jani.
>
>>
>> Also looks like the CHV spec was forked from VLV before someone fixed
>> the prepare count to be 6 bits, which seems to be what my VLV actually
>> has as well. Not the first time the VLV docs are more up to date than
>> the CHV ones unfortunately.
>>
>>> 
>>> Define the max values based on the platform. Also define them based on
>>> the register definitions instead of duplicating information.
>>> 
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106130
>>> Cc: stable@xxxxxxxxxxxxxxx
>>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg.h      | 11 +++++++----
>>>  drivers/gpu/drm/i915/intel_dsi_vbt.c | 31 +++++++++++++++++++------------
>>>  2 files changed, 26 insertions(+), 16 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index fb106026a1f4..f4435a13b757 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -9547,14 +9547,17 @@ enum skl_power_gate {
>>>  #define _MIPIA_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb080)
>>>  #define _MIPIC_DPHY_PARAM		(dev_priv->mipi_mmio_base + 0xb880)
>>>  #define MIPI_DPHY_PARAM(port)		_MMIO_MIPI(port, _MIPIA_DPHY_PARAM, _MIPIC_DPHY_PARAM)
>>> -#define  EXIT_ZERO_COUNT_SHIFT				24
>>>  #define  EXIT_ZERO_COUNT_MASK				(0x3f << 24)
>>> -#define  TRAIL_COUNT_SHIFT				16
>>> +#define  EXIT_ZERO_COUNT_MASK_CHV			(0xff << 24)
>>> +#define  EXIT_ZERO_COUNT_SHIFT				24
>>>  #define  TRAIL_COUNT_MASK				(0x1f << 16)
>>> -#define  CLK_ZERO_COUNT_SHIFT				8
>>> +#define  TRAIL_COUNT_MASK_CHV				(0xff << 16)
>>> +#define  TRAIL_COUNT_SHIFT				16
>>>  #define  CLK_ZERO_COUNT_MASK				(0xff << 8)
>>> -#define  PREPARE_COUNT_SHIFT				0
>>> +#define  CLK_ZERO_COUNT_SHIFT				8
>>>  #define  PREPARE_COUNT_MASK				(0x3f << 0)
>>> +#define  PREPARE_COUNT_MASK_CHV				(0xff << 0)
>>> +#define  PREPARE_COUNT_SHIFT				0
>>>  
>>>  /* bits 31:0 */
>>>  #define _MIPIA_DBI_BW_CTRL		(dev_priv->mipi_mmio_base + 0xb084)
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi_vbt.c b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>>> index 4d6ffa7b3e7b..8d3dea693840 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi_vbt.c
>>> +++ b/drivers/gpu/drm/i915/intel_dsi_vbt.c
>>> @@ -41,10 +41,17 @@
>>>  #define MIPI_VIRTUAL_CHANNEL_SHIFT	1
>>>  #define MIPI_PORT_SHIFT			3
>>>  
>>> -#define PREPARE_CNT_MAX		0x3F
>>> -#define EXIT_ZERO_CNT_MAX	0x3F
>>> -#define CLK_ZERO_CNT_MAX	0xFF
>>> -#define TRAIL_CNT_MAX		0x1F
>>> +#define EXIT_ZERO_CNT_MAX(dev_priv) \
>>> +	((IS_VALLEYVIEW(dev_priv) ? EXIT_ZERO_COUNT_MASK : EXIT_ZERO_COUNT_MASK_CHV) >> EXIT_ZERO_COUNT_SHIFT)
>>> +
>>> +#define TRAIL_CNT_MAX(dev_priv) \
>>> +	((IS_VALLEYVIEW(dev_priv) ? TRAIL_COUNT_MASK : TRAIL_COUNT_MASK_CHV) >> TRAIL_COUNT_SHIFT)
>>> +
>>> +#define CLK_ZERO_CNT_MAX(dev_priv) \
>>> +	(CLK_ZERO_COUNT_MASK >> CLK_ZERO_COUNT_SHIFT)
>>> +
>>> +#define PREPARE_CNT_MAX(dev_priv)					\
>>> +	((IS_VALLEYVIEW(dev_priv) ? PREPARE_COUNT_MASK : PREPARE_COUNT_MASK_CHV) >> PREPARE_COUNT_SHIFT)
>>>  
>>>  #define NS_KHZ_RATIO 1000000
>>>  
>>> @@ -647,9 +654,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>>>  	/* prepare count */
>>>  	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num * mul);
>>>  
>>> -	if (prepare_cnt > PREPARE_CNT_MAX) {
>>> +	if (prepare_cnt > PREPARE_CNT_MAX(dev_priv)) {
>>>  		DRM_DEBUG_KMS("prepare count too high %u\n", prepare_cnt);
>>> -		prepare_cnt = PREPARE_CNT_MAX;
>>> +		prepare_cnt = PREPARE_CNT_MAX(dev_priv);
>>>  	}
>>>  
>>>  	/* exit zero count */
>>> @@ -667,9 +674,9 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>>>  	if (exit_zero_cnt < (55 * ui_den / ui_num) && (55 * ui_den) % ui_num)
>>>  		exit_zero_cnt += 1;
>>>  
>>> -	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX) {
>>> +	if (exit_zero_cnt > EXIT_ZERO_CNT_MAX(dev_priv)) {
>>>  		DRM_DEBUG_KMS("exit zero count too high %u\n", exit_zero_cnt);
>>> -		exit_zero_cnt = EXIT_ZERO_CNT_MAX;
>>> +		exit_zero_cnt = EXIT_ZERO_CNT_MAX(dev_priv);
>>>  	}
>>>  
>>>  	/* clk zero count */
>>> @@ -677,18 +684,18 @@ bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
>>>  				(tclk_prepare_clkzero -	ths_prepare_ns)
>>>  				* ui_den, ui_num * mul);
>>>  
>>> -	if (clk_zero_cnt > CLK_ZERO_CNT_MAX) {
>>> +	if (clk_zero_cnt > CLK_ZERO_CNT_MAX(dev_priv)) {
>>>  		DRM_DEBUG_KMS("clock zero count too high %u\n", clk_zero_cnt);
>>> -		clk_zero_cnt = CLK_ZERO_CNT_MAX;
>>> +		clk_zero_cnt = CLK_ZERO_CNT_MAX(dev_priv);
>>>  	}
>>>  
>>>  	/* trail count */
>>>  	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
>>>  	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, ui_num * mul);
>>>  
>>> -	if (trail_cnt > TRAIL_CNT_MAX) {
>>> +	if (trail_cnt > TRAIL_CNT_MAX(dev_priv)) {
>>>  		DRM_DEBUG_KMS("trail count too high %u\n", trail_cnt);
>>> -		trail_cnt = TRAIL_CNT_MAX;
>>> +		trail_cnt = TRAIL_CNT_MAX(dev_priv);
>>>  	}
>>>  
>>>  	/* B080 */
>>> -- 
>>> 2.11.0

-- 
Jani Nikula, Intel Open Source Technology Center



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]