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