On Tue, Jun 18, 2019 at 12:59:59PM -0700, José Roberto de Souza wrote:
> From: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
>
> EHL has 2 additional steps in the DSI sequence, this is one of then
> the lane latency optimization for DW1.
>
> BSpec: 20597
> Cc: Uma Shankar <uma.shankar@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 11 +++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 74448e6bf749..ee85428b309f 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -403,6 +403,17 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
> tmp &= ~FRC_LATENCY_OPTIM_MASK;
> tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
> I915_WRITE(ICL_PORT_TX_DW2_GRP(port), tmp);
> + /* For EHL set latency optimization for PCS_DW1 lanes */
> + if (IS_ELKHARTLAKE(dev_priv)) {
> + tmp = I915_READ(ICL_PORT_PCS_DW1_AUX(port));
> + tmp &= ~LATENCY_OPTIM_MASK;
> + tmp |= LATENCY_OPTIM_VAL(0);
> + I915_WRITE(ICL_PORT_PCS_DW1_AUX(port), tmp);
> + tmp = I915_READ(ICL_PORT_PCS_DW1_LN0(port));
> + tmp &= ~LATENCY_OPTIM_MASK;
> + tmp |= LATENCY_OPTIM_VAL(0x1);
> + I915_WRITE(ICL_PORT_PCS_DW1_GRP(port), tmp);
> + }
Minor nitpick, but these sequences might be slightly easier to read if
there was a blank line separating each R/M/W chunk.
The changes here look correct according to the description on bspec page
20597 although it looks like the bspec authors forgot to update the
'Valid Values' section for these bits on page 20398; not sure if you
want to file a bspec defect about that or not.
> }
>
> }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d6483b5dc8e5..1f2c3ebdf87b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1896,6 +1896,8 @@ enum i915_power_well_id {
> #define ICL_PORT_PCS_DW1_GRP(port) _MMIO(_ICL_PORT_PCS_DW_GRP(1, port))
> #define ICL_PORT_PCS_DW1_LN0(port) _MMIO(_ICL_PORT_PCS_DW_LN(1, 0, port))
> #define COMMON_KEEPER_EN (1 << 26)
> +#define LATENCY_OPTIM_MASK (0x3 << 2)
> +#define LATENCY_OPTIM_VAL(x) ((x) << 2)
Should we try to include part of the name of the register in these
definitions (e.g., DW1_LATENCY_OPTIM)? I'm not sure if we should worry
about people mixing up these vs the FRC_LATENCY_OPTIM defines farther
down for the TX_DW2 register.
Up to you on whether you think it's worth clarifying the naming. Either
way,
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>
> /* CNL/ICL Port TX registers */
> #define _CNL_PORT_TX_AE_GRP_OFFSET 0x162340
> --
> 2.22.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
