[Intel-gfx] [PATCH v2 3/3] drm/i915: Get panel_type from OpRegion panel details

[Ville Syrjälä]

On Mon, Apr 11, 2016 at 11:09:57AM +0300, Jani Nikula wrote:
> On Mon, 11 Apr 2016, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > We've had problems on several occasions with using the panel type
> > from the VBT block 40. Usually it seems to be 2, which often
> > doesn't give us the correct timings for the panel. After some
> > more digging I found a way to get a panel type via the OpRegion
> > SWSCI GBDA "Get Panel Details" method. Let's try to use it.
> >
> > The spec has this to say about the output:
> > "Bits [15:8] - Panel Type
> >  Bits contain the panel type user setting from CMOS
> >  00h = Not Valid, use default Panel Type & Timings from VBT
> >  01h - 0Fh = Panel Number"
> >
> > Another version of the spec lists the valid range as 1-16, which makes
> > more sense since VBT supports 16 panels. Based on actual results
> > from Rob's G45, 1-16 is what we need to accept.
> >
> > The other bits in the output don't look relevant for the problem at
> > hand.
> >
> > The input is specified as:
> > "Bits [31:4] - Reserved
> >  Reserved (must be zero)
> >  Bits [3:0] - Panel Number
> >  These bits contain the sequential index of Panel, starting at 0 and
> >  counting upwards from the first integrated Internal Flat-Panel Display
> >  Encoder present, and then from the first external Display Encoder
> >  (e.g., S/DVO-B then S/DVO-C) which supports Internal Flat-Panels.
> >  0h - 0Fh = Panel number"
> >
> > For now I've just hardcoded the input panel number as 0. That would seem
> > like a decent choise for LVDS. Not so sure about eDP when port != A.
> >
> > v2: Accept values 1-16
> >     Filter out bogus results in opregion code (Jani)
> >     Add debug logging for all the different branches (Jani)
> >
> > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > Cc: Rob Kramer <rob at solution-space.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> So I don't know if this is the right thing to do or not, and we'll only
> find out with testing, but the code looks good to me.
> 
> With that caveat,
> 
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>

Entire series pushed to dinq. Thanks for the reviews and testing.

> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h       |  5 +++++
> >  drivers/gpu/drm/i915/intel_bios.c     | 20 +++++++++++++++-----
> >  drivers/gpu/drm/i915/intel_opregion.c | 28 ++++++++++++++++++++++++++++
> >  3 files changed, 48 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 53ace572b292..42234e17b789 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3421,6 +3421,7 @@ extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >  					 bool enable);
> >  extern int intel_opregion_notify_adapter(struct drm_device *dev,
> >  					 pci_power_t state);
> > +extern int intel_opregion_get_panel_type(struct drm_device *dev);
> >  #else
> >  static inline int intel_opregion_setup(struct drm_device *dev) { return 0; }
> >  static inline void intel_opregion_init(struct drm_device *dev) { return; }
> > @@ -3436,6 +3437,10 @@ intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
> >  {
> >  	return 0;
> >  }
> > +static inline int intel_opregion_get_panel_type(struct drm_device *dev)
> > +{
> > +	return -ENODEV;
> > +}
> >  #endif
> >  
> >  /* intel_acpi.c */
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index d595ca30a7e1..e72dd9a8d6bf 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -205,19 +205,29 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
> >  	struct drm_display_mode *panel_fixed_mode;
> >  	int panel_type;
> >  	int drrs_mode;
> > +	int ret;
> >  
> >  	lvds_options = find_section(bdb, BDB_LVDS_OPTIONS);
> >  	if (!lvds_options)
> >  		return;
> >  
> >  	dev_priv->vbt.lvds_dither = lvds_options->pixel_dither;
> > -	if (lvds_options->panel_type > 0xf) {
> > -		DRM_DEBUG_KMS("Invalid VBT panel type 0x%x\n",
> > -			      lvds_options->panel_type);
> > -		return;
> > +
> > +	ret = intel_opregion_get_panel_type(dev_priv->dev);
> > +	if (ret >= 0) {
> > +		WARN_ON(ret > 0xf);
> > +		panel_type = ret;
> > +		DRM_DEBUG_KMS("Panel type: %d (OpRegion)\n", panel_type);
> > +	} else {
> > +		if (lvds_options->panel_type > 0xf) {
> > +			DRM_DEBUG_KMS("Invalid VBT panel type 0x%x\n",
> > +				      lvds_options->panel_type);
> > +			return;
> > +		}
> > +		panel_type = lvds_options->panel_type;
> > +		DRM_DEBUG_KMS("Panel type: %d (VBT)\n", panel_type);
> >  	}
> >  
> > -	panel_type = lvds_options->panel_type;
> >  	dev_priv->vbt.panel_type = panel_type;
> >  
> >  	drrs_mode = (lvds_options->dps_panel_type_bits
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index c15718b4862a..d3c4945a0e36 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -1024,3 +1024,31 @@ err_out:
> >  	memunmap(base);
> >  	return err;
> >  }
> > +
> > +int
> > +intel_opregion_get_panel_type(struct drm_device *dev)
> > +{
> > +	u32 panel_details;
> > +	int ret;
> > +
> > +	ret = swsci(dev, SWSCI_GBDA_PANEL_DETAILS, 0x0, &panel_details);
> > +	if (ret) {
> > +		DRM_DEBUG_KMS("Failed to get panel details from OpRegion (%d)\n",
> > +			      ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = (panel_details >> 8) & 0xff;
> > +	if (ret > 0x10) {
> > +		DRM_DEBUG_KMS("Invalid OpRegion panel type 0x%x\n", ret);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* fall back to VBT panel type? */
> > +	if (ret == 0x0) {
> > +		DRM_DEBUG_KMS("No panel type in OpRegion\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	return ret - 1;
> > +}
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC