[Intel-gfx] [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT
From: Ville Syrjälä VBT appears to have two (or possibly three) ways to indicate the panel rotation. The first is in the MIPI config block, but that apparenly usually (maybe always?) indicates 0 degrees despite the actual panel orientation. The second way to indicate this is in the general features block, which can just indicate whether 180 degress rotation is used. The third might be a separate rotation data block, but that is not at all documented so who knows what it may contain. Let's try the first two. We first try the DSI specicic VBT information, and it it doesn't look trustworthy (ie. indicates 0 degrees) we fall back to the 180 degree thing. Just to avoid too many changes in one go we shall also keep the hardware readout path for now. If this works for more than just my VLV FFRD the question becomes how many of the panel orientation quirks are now redundant? Cc: Hans de Goede Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_bios.c | 31 +++ drivers/gpu/drm/i915/vlv_dsi.c| 11 ++- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3017ef037fed..115d5963e5a6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1102,6 +1102,7 @@ struct intel_vbt_data { unsigned int panel_type:4; int lvds_ssc_freq; unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */ + enum drm_panel_orientation orientation; enum drrs_support_type drrs_type; @@ -1147,6 +1148,7 @@ struct intel_vbt_data { u8 *data; const u8 *sequence[MIPI_SEQ_MAX]; u8 *deassert_seq; /* Used by fixup_mipi_sequences() */ + enum drm_panel_orientation orientation; } dsi; int crt_ddc_pin; diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 1faa494e2bc9..a4bfd92212df 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private *dev_priv, intel_bios_ssc_frequency(dev_priv, general->ssc_freq); dev_priv->vbt.display_clock_mode = general->display_clock_mode; dev_priv->vbt.fdi_rx_polarity_inverted = general->fdi_rx_polarity_inverted; + if (bdb->version >= 181) { + dev_priv->vbt.orientation = general->rotate_180 ? + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP : + DRM_MODE_PANEL_ORIENTATION_NORMAL; + } else { + dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; + } DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d fdi_rx_polarity_inverted %d\n", dev_priv->vbt.int_tv_support, dev_priv->vbt.int_crt_support, @@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv, parse_dsi_backlight_ports(dev_priv, bdb->version, port); + /* FIXME is the 90 vs. 270 correct? */ + switch (config->rotation) { + case ENABLE_ROTATION_0: + /* +* Most (all?) VBTs claim 0 degrees despite having +* an upside down panel, thus we do not trust this. +*/ + dev_priv->vbt.dsi.orientation = + DRM_MODE_PANEL_ORIENTATION_UNKNOWN; + break; + case ENABLE_ROTATION_90: + dev_priv->vbt.dsi.orientation = + DRM_MODE_PANEL_ORIENTATION_RIGHT_UP; + break; + case ENABLE_ROTATION_180: + dev_priv->vbt.dsi.orientation = + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP; + break; + case ENABLE_ROTATION_270: + dev_priv->vbt.dsi.orientation = + DRM_MODE_PANEL_ORIENTATION_LEFT_UP; + break; + } + /* We have mandatory mipi config blocks. Initialize as generic panel */ dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID; } diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c index 893839ea0ff9..a3119ec3189a 100644 --- a/drivers/gpu/drm/i915/vlv_dsi.c +++ b/drivers/gpu/drm/i915/vlv_dsi.c @@ -1704,7 +1704,8 @@ vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector) return orientation; } -static int intel_dsi_get_panel_orientation(struct intel_connector *connector) +static enum drm_panel_orientation +intel_dsi_get_panel_orientation(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); enum drm_panel_orientation orientation; @@ -1715,6 +1716,14 @@ static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT
On Fri, 19 Oct 2018, Ville Syrjala wrote: > From: Ville Syrjälä > > VBT appears to have two (or possibly three) ways to indicate the panel > rotation. The first is in the MIPI config block, but that apparenly > usually (maybe always?) indicates 0 degrees despite the actual panel > orientation. The second way to indicate this is in the general features > block, which can just indicate whether 180 degress rotation is used. > The third might be a separate rotation data block, but that is not > at all documented so who knows what it may contain. > > Let's try the first two. We first try the DSI specicic VBT > information, and it it doesn't look trustworthy (ie. indicates > 0 degrees) we fall back to the 180 degree thing. Just to avoid too > many changes in one go we shall also keep the hardware readout path > for now. > > If this works for more than just my VLV FFRD the question becomes > how many of the panel orientation quirks are now redundant? > > Cc: Hans de Goede > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_bios.c | 31 +++ > drivers/gpu/drm/i915/vlv_dsi.c| 11 ++- > 3 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 3017ef037fed..115d5963e5a6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1102,6 +1102,7 @@ struct intel_vbt_data { > unsigned int panel_type:4; > int lvds_ssc_freq; > unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */ > + enum drm_panel_orientation orientation; > > enum drrs_support_type drrs_type; > > @@ -1147,6 +1148,7 @@ struct intel_vbt_data { > u8 *data; > const u8 *sequence[MIPI_SEQ_MAX]; > u8 *deassert_seq; /* Used by fixup_mipi_sequences() */ > + enum drm_panel_orientation orientation; Would be nice to expose just one orientation field, but I guess we need the duplication... *sigh* > } dsi; > > int crt_ddc_pin; > diff --git a/drivers/gpu/drm/i915/intel_bios.c > b/drivers/gpu/drm/i915/intel_bios.c > index 1faa494e2bc9..a4bfd92212df 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private *dev_priv, > intel_bios_ssc_frequency(dev_priv, general->ssc_freq); > dev_priv->vbt.display_clock_mode = general->display_clock_mode; > dev_priv->vbt.fdi_rx_polarity_inverted = > general->fdi_rx_polarity_inverted; > + if (bdb->version >= 181) { > + dev_priv->vbt.orientation = general->rotate_180 ? > + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP : > + DRM_MODE_PANEL_ORIENTATION_NORMAL; > + } else { > + dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > + } > DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support > %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d > fdi_rx_polarity_inverted %d\n", > dev_priv->vbt.int_tv_support, > dev_priv->vbt.int_crt_support, > @@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv, > > parse_dsi_backlight_ports(dev_priv, bdb->version, port); > > + /* FIXME is the 90 vs. 270 correct? */ *sigh* Your guess is as good as mine. > + switch (config->rotation) { > + case ENABLE_ROTATION_0: > + /* > + * Most (all?) VBTs claim 0 degrees despite having > + * an upside down panel, thus we do not trust this. > + */ > + dev_priv->vbt.dsi.orientation = > + DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > + break; > + case ENABLE_ROTATION_90: > + dev_priv->vbt.dsi.orientation = > + DRM_MODE_PANEL_ORIENTATION_RIGHT_UP; > + break; > + case ENABLE_ROTATION_180: > + dev_priv->vbt.dsi.orientation = > + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP; > + break; > + case ENABLE_ROTATION_270: > + dev_priv->vbt.dsi.orientation = > + DRM_MODE_PANEL_ORIENTATION_LEFT_UP; > + break; > + } > + > /* We have mandatory mipi config blocks. Initialize as generic panel */ > dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID; > } > diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c > index 893839ea0ff9..a3119ec3189a 100644 > --- a/drivers/gpu/drm/i915/vlv_dsi.c > +++ b/drivers/gpu/drm/i915/vlv_dsi.c > @@ -1704,7 +1704,8 @@ vlv_dsi_get_hw_panel_orientation(struct intel_connector > *connector) > return orientation; > } > > -static int intel_dsi_get_panel_orientation(struct intel_connector *connector) > +static enum drm_panel_orientation > +intel_dsi_ge
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT
On Mon, Oct 22, 2018 at 04:07:27PM +0300, Jani Nikula wrote: > On Fri, 19 Oct 2018, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > VBT appears to have two (or possibly three) ways to indicate the panel > > rotation. The first is in the MIPI config block, but that apparenly > > usually (maybe always?) indicates 0 degrees despite the actual panel > > orientation. The second way to indicate this is in the general features > > block, which can just indicate whether 180 degress rotation is used. > > The third might be a separate rotation data block, but that is not > > at all documented so who knows what it may contain. > > > > Let's try the first two. We first try the DSI specicic VBT > > information, and it it doesn't look trustworthy (ie. indicates > > 0 degrees) we fall back to the 180 degree thing. Just to avoid too > > many changes in one go we shall also keep the hardware readout path > > for now. > > > > If this works for more than just my VLV FFRD the question becomes > > how many of the panel orientation quirks are now redundant? > > > > Cc: Hans de Goede > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_bios.c | 31 +++ > > drivers/gpu/drm/i915/vlv_dsi.c| 11 ++- > > 3 files changed, 43 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 3017ef037fed..115d5963e5a6 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1102,6 +1102,7 @@ struct intel_vbt_data { > > unsigned int panel_type:4; > > int lvds_ssc_freq; > > unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */ > > + enum drm_panel_orientation orientation; > > > > enum drrs_support_type drrs_type; > > > > @@ -1147,6 +1148,7 @@ struct intel_vbt_data { > > u8 *data; > > const u8 *sequence[MIPI_SEQ_MAX]; > > u8 *deassert_seq; /* Used by fixup_mipi_sequences() */ > > + enum drm_panel_orientation orientation; > > Would be nice to expose just one orientation field, but I guess we need > the duplication... *sigh* There is maybe a theoretical concern about systems with mixed panel types (eDP+DSI for example) where we might need different rotation values for each type. But if we assume that this stuff only matters for DSI panels then we could leave the policy of "which field do we prefer" in the VBT parsing code. The whole thing is a mess anyway. There really should just be a per-panel orientation knob in the VBT. > > > } dsi; > > > > int crt_ddc_pin; > > diff --git a/drivers/gpu/drm/i915/intel_bios.c > > b/drivers/gpu/drm/i915/intel_bios.c > > index 1faa494e2bc9..a4bfd92212df 100644 > > --- a/drivers/gpu/drm/i915/intel_bios.c > > +++ b/drivers/gpu/drm/i915/intel_bios.c > > @@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private > > *dev_priv, > > intel_bios_ssc_frequency(dev_priv, general->ssc_freq); > > dev_priv->vbt.display_clock_mode = general->display_clock_mode; > > dev_priv->vbt.fdi_rx_polarity_inverted = > > general->fdi_rx_polarity_inverted; > > + if (bdb->version >= 181) { > > + dev_priv->vbt.orientation = general->rotate_180 ? > > + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP : > > + DRM_MODE_PANEL_ORIENTATION_NORMAL; > > + } else { > > + dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > > + } > > DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support > > %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d > > fdi_rx_polarity_inverted %d\n", > > dev_priv->vbt.int_tv_support, > > dev_priv->vbt.int_crt_support, > > @@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv, > > > > parse_dsi_backlight_ports(dev_priv, bdb->version, port); > > > > + /* FIXME is the 90 vs. 270 correct? */ > > *sigh* Your guess is as good as mine. > > > + switch (config->rotation) { > > + case ENABLE_ROTATION_0: > > + /* > > +* Most (all?) VBTs claim 0 degrees despite having > > +* an upside down panel, thus we do not trust this. > > +*/ > > + dev_priv->vbt.dsi.orientation = > > + DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > > + break; > > + case ENABLE_ROTATION_90: > > + dev_priv->vbt.dsi.orientation = > > + DRM_MODE_PANEL_ORIENTATION_RIGHT_UP; > > + break; > > + case ENABLE_ROTATION_180: > > + dev_priv->vbt.dsi.orientation = > > + DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP; > > + break; > > + case ENABLE_ROTATION_270: > > + dev_priv->vbt.dsi.orientation = > > + DRM_MODE_PANEL_ORIENTATION_LEFT_UP; > > + break; > > + } > > + > > /* We have