On Mon, Apr 11, 2016 at 11:09:57AM +0300, Jani Nikula wrote: > On Mon, 11 Apr 2016, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxx> > > Cc: Rob Kramer <rob@xxxxxxxxxxxxxxxxxx> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825 > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxx> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
