[Intel-gfx] [PATCH v2] drm/i915/dsi: Fix swapping of MIPI_SEQ_DEASSERT_RESET / MIPI_SEQ_ASSERT_RESET

[Ville Syrjälä]

On Fri, Dec 02, 2016 at 04:01:28PM +0100, Hans de Goede wrote:
> Looking at the ADF code from the Android kernel sources for a
> cherrytrail tablet I noticed that it is calling the
> MIPI_SEQ_ASSERT_RESET sequence from the panel prepare hook.
> 
> Until commit b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences
> in panel prepare/unprepare hooks") the mainline i915 code was doing the
> same. That commits effectively swaps the calling of MIPI_SEQ_ASSERT_RESET /
> MIPI_SEQ_DEASSERT_RESET.
> 
> Looking at the naming of the sequences that is the right thing to do,
> but the problem is, that the old mainline code and the ADF code was
> actually calling the right sequence (tested on a cube iwork8 air tablet),
> and the swapping of the calling breaks things.
> 
> This breakage was likely not noticed in testing because on cherrytrail,
> currently chv_exec_gpio ends up disabling the gpio pins rather then
> setting them (this is fixed in the next patch in this patch-set).
> 
> This commit fixes the swapping by fixing MIPI_SEQ_ASSERT/DEASSERT_RESET's
> places in the enum defining them, so that their (new) names match their
> actual use.
> 
> Fixes: b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences...")
> Cc: Jani Nikula <jani.nikula at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> Acked-by: Jani Nikula <jani.nikula at intel.com>
> ---
> Changes in v2:
> -Add a comment to the enum explaining that the assert/reassert names
>  are swapped in the spec

Pushed to dinq. Thanks for the patch.

I sucked in the changelog again. In the future please include it in
the actual commit message, cause that's how we like it.

> ---
>  drivers/gpu/drm/i915/intel_bios.h          | 12 +++++++++---
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  4 ++--
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 8405b5a..7e3545f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -46,14 +46,20 @@ struct edp_power_seq {
>  	u16 t11_t12;
>  } __packed;
>  
> -/* MIPI Sequence Block definitions */
> +/*
> + * MIPI Sequence Block definitions
> + *
> + * Note the VBT spec has AssertReset / DeassertReset swapped from their
> + * usual naming, we use the proper names here to avoid confusion when
> + * reading the code.
> + */
>  enum mipi_seq {
>  	MIPI_SEQ_END = 0,
> -	MIPI_SEQ_ASSERT_RESET,
> +	MIPI_SEQ_DEASSERT_RESET,	/* Spec says MipiAssertResetPin */
>  	MIPI_SEQ_INIT_OTP,
>  	MIPI_SEQ_DISPLAY_ON,
>  	MIPI_SEQ_DISPLAY_OFF,
> -	MIPI_SEQ_DEASSERT_RESET,
> +	MIPI_SEQ_ASSERT_RESET,		/* Spec says MipiDeassertResetPin */
>  	MIPI_SEQ_BACKLIGHT_ON,		/* sequence block v2+ */
>  	MIPI_SEQ_BACKLIGHT_OFF,		/* sequence block v2+ */
>  	MIPI_SEQ_TEAR_ON,		/* sequence block v2+ */
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 0d8ff00..579d2f5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -376,11 +376,11 @@ static const fn_mipi_elem_exec exec_elem[] = {
>   */
>  
>  static const char * const seq_name[] = {
> -	[MIPI_SEQ_ASSERT_RESET] = "MIPI_SEQ_ASSERT_RESET",
> +	[MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
>  	[MIPI_SEQ_INIT_OTP] = "MIPI_SEQ_INIT_OTP",
>  	[MIPI_SEQ_DISPLAY_ON] = "MIPI_SEQ_DISPLAY_ON",
>  	[MIPI_SEQ_DISPLAY_OFF]  = "MIPI_SEQ_DISPLAY_OFF",
> -	[MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
> +	[MIPI_SEQ_ASSERT_RESET] = "MIPI_SEQ_ASSERT_RESET",
>  	[MIPI_SEQ_BACKLIGHT_ON] = "MIPI_SEQ_BACKLIGHT_ON",
>  	[MIPI_SEQ_BACKLIGHT_OFF] = "MIPI_SEQ_BACKLIGHT_OFF",
>  	[MIPI_SEQ_TEAR_ON] = "MIPI_SEQ_TEAR_ON",
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel OTC