On Fri, Apr 08, 2016 at 06:01:41PM +0300, Jani Nikula wrote: > On Fri, 08 Apr 2016, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > On Fri, Apr 08, 2016 at 05:50:06PM +0300, Jani Nikula wrote: > >> On Fri, 08 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" > >> > > >> > 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. > >> > >> Frankly, I didn't understand the point of the input parameter for > >> figuring out the output. The spec is written for someone who already > >> knows what they're doing... > >> > >> > > >> > Cc: Rob Kramer <rob@xxxxxxxxxxxxxxxxxx> > >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825 > >> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> > --- > >> > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > >> > drivers/gpu/drm/i915/intel_bios.c | 13 ++++++++++--- > >> > drivers/gpu/drm/i915/intel_opregion.c | 13 +++++++++++++ > >> > 3 files changed, 28 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> > index 53ace572b292..533263a7a12d 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 0; > >> > +} > >> > #endif > >> > > >> > /* intel_acpi.c */ > >> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > >> > index d9568be4b33b..718e49931b5f 100644 > >> > --- a/drivers/gpu/drm/i915/intel_bios.c > >> > +++ b/drivers/gpu/drm/i915/intel_bios.c > >> > @@ -205,16 +205,23 @@ 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) > >> > - return; > >> > > >> > - panel_type = lvds_options->panel_type; > >> > + ret = intel_opregion_get_panel_type(dev_priv->dev); > >> > + if (ret >= 0x1 && ret <= 0xf) { > >> > >> Undecided whether I'd like to have this check within > >> intel_opregion_get_panel_type(). Just make that return 0 for "use vbt" > >> (and CONFIG_ACPI=n), and valid stuff otherwise? *shrug* > > > > We don't compile intel_opregion.c when ACPI==n. > > Yeah I meant the static inline stub in i915_drv.h for ACPI=n. Which does > return 0. I'm a bit lost here. Oh, maybe the ACPI=n note just confused me. Did you just mean that I should do the 0x1-0xf check in intel_opregion_get_panel_type() already? > >> > + panel_type = ret; > >> > + } else { > >> > + if (lvds_options->panel_type > 0xf) > >> > + return; > >> > + panel_type = lvds_options->panel_type; > >> > + } > >> > + > >> > >> If this actually works, I'd like to have some debug logging for pretty > >> much all the code paths here. > > > > Yeah that would be nice for figuring out where the information came > > from or why it was rejected. > > > >> > >> Other than that, seems fine. > >> > >> BR, > >> Jani. > >> > >> > 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..5e17fa5dc869 100644 > >> > --- a/drivers/gpu/drm/i915/intel_opregion.c > >> > +++ b/drivers/gpu/drm/i915/intel_opregion.c > >> > @@ -1024,3 +1024,16 @@ 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) > >> > + return ret; > >> > + > >> > + return (panel_details >> 8) & 0xff; > >> > +} > >> > >> -- > >> Jani Nikula, Intel Open Source Technology Center > > -- > 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