Re: [Intel-gfx] [PATCH] drm/i915: Ignore OpRegion panel type except on select machines

2016-09-14 Thread James Hogan
On 13 September 2016 at 10:22,   wrote:
> From: Ville Syrjälä 
>
> Turns out
> commit a05628195a0d ("drm/i915: Get panel_type from OpRegion panel
> details") has regressed quite a few machines. So it looks like we
> can't use the panel type from OpRegion on all systems, and yet we
> absolutely must use it on some specific systems.
>
> Despite trying, I was unable to find any automagic way to determine
> if the OpRegion panel type is respectable or not. The only glimmer
> of hope I had was bit 8 in the SCIC response, but that turned out to
> not work either (it was always 0 on both types of systems).
>
> So, to fix the regressions without breaking the machine we know to need
> the OpRegion panel type, let's just add a quirk for this. Only specific
> machines known to require the OpRegion panel type will therefore use
> it. Everyone else will fall bck to the VBT panel type.
>
> The only known machine so far is a "Conrac GmbH IX45GM2". The PCI
> subsystem ID on this machine is just a generic 8086:2a42, so of no use.
> Instead we'll go with a DMI match.
>
> I suspect we can now also revert
> commit aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")
> but let's leave that to a separate patch.
>
> v2: Do the DMI match in the opregion code directly, as dev_priv->quirks
> gets populated too late
>
> Cc: Rob Kramer 
> Cc: Martin van Es 
> Cc: Andrea Arcangeli 
> Cc: Dave Airlie 
> Cc: Marco Krüger 
> Cc: Sean Greenslade 
> Cc: Trudy Tective 
> Cc: Robin Müller 
> Cc: Alexander Kobel 
> Cc: Alexey Shumitsky 
> Cc: Emil Andersen Lauridsen 
> Cc: oceans...@gmail.com
> Cc: James Hogan 
> Cc: James Bottomley 
> Cc: sta...@vger.kernel.org
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2016-August/105545.html
> References: 
> https://lists.freedesktop.org/archives/dri-devel/2016-August/116888.html
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97060
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97443
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97363
> Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
> Tested-by: Marco Krüger 
> Tested-by: Alexey Shumitsky 
> Tested-by: Sean Greenslade 
> Tested-by: Emil Andersen Lauridsen 
> Tested-by: Robin Müller 
> Tested-by: oceans...@gmail.com
> Signed-off-by: Ville Syrjälä 

That works for me too on XPS13. Flickering screen brightness gone, and
using acpi backlight rather than intel backlight, like before
a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details").

Tested-by: James Hogan 

Thanks!
James

> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index adca262d591a..7acbbbf97833 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -1047,6 +1047,23 @@ err_out:
> return err;
>  }
>
> +static int intel_use_opregion_panel_type_callback(const struct dmi_system_id 
> *id)
> +{
> +   DRM_INFO("Using panel type from OpRegion on %s\n", id->ident);
> +   return 1;
> +}
> +
> +static const struct dmi_system_id intel_use_opregion_panel_type[] = {
> +   {
> +   .callback = intel_use_opregion_panel_type_callback,
> +   .ident = "Conrac GmbH IX45GM2",
> +   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Conrac GmbH"),
> +   DMI_MATCH(DMI_PRODUCT_NAME, "IX45GM2"),
> +   },
> +   },
> +   { }
> +};
> +
>  int
>  intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
>  {
> @@ -1073,6 +1090,16 @@ intel_opregion_get_panel_type(struct drm_i915_private 
> *dev_priv)
> }
>
> /*
> +* So far we know that some machined must use it, others must not use 
> it.
> +* There doesn't seem to be any way to determine which way to go, 
> except
> +* via a quirk list :(
> +*/
> +   if (!dmi_check_system(intel_use_opregion_panel_type)) {
> +   DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret - 1);
> +   return -ENODEV;
> +   }
> +
> +   /*
>  * FIXME On Dell XPS 13 9350 the OpRegion panel type (0) gives us
>  * low vswing for eDP, 

Re: [Intel-gfx] [PATCH] drm/i915: Ignore OpRegion panel type except on select machines

2016-09-14 Thread Ville Syrjälä
On Tue, Sep 13, 2016 at 12:37:16PM +0300, Jani Nikula wrote:
> On Tue, 13 Sep 2016, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> >
> > Turns out
> > commit a05628195a0d ("drm/i915: Get panel_type from OpRegion panel
> > details") has regressed quite a few machines. So it looks like we
> > can't use the panel type from OpRegion on all systems, and yet we
> > absolutely must use it on some specific systems.
> >
> > Despite trying, I was unable to find any automagic way to determine
> > if the OpRegion panel type is respectable or not. The only glimmer
> > of hope I had was bit 8 in the SCIC response, but that turned out to
> > not work either (it was always 0 on both types of systems).
> >
> > So, to fix the regressions without breaking the machine we know to need
> > the OpRegion panel type, let's just add a quirk for this. Only specific
> > machines known to require the OpRegion panel type will therefore use
> > it. Everyone else will fall bck to the VBT panel type.
> >
> > The only known machine so far is a "Conrac GmbH IX45GM2". The PCI
> > subsystem ID on this machine is just a generic 8086:2a42, so of no use.
> > Instead we'll go with a DMI match.
> >
> > I suspect we can now also revert
> > commit aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")
> > but let's leave that to a separate patch.
> >
> > v2: Do the DMI match in the opregion code directly, as dev_priv->quirks
> > gets populated too late
> >
> > Cc: Rob Kramer 
> > Cc: Martin van Es 
> > Cc: Andrea Arcangeli 
> > Cc: Dave Airlie 
> > Cc: Marco Krüger 
> > Cc: Sean Greenslade 
> > Cc: Trudy Tective 
> > Cc: Robin Müller 
> > Cc: Alexander Kobel 
> > Cc: Alexey Shumitsky 
> > Cc: Emil Andersen Lauridsen 
> > Cc: oceans...@gmail.com
> > Cc: James Hogan 
> > Cc: James Bottomley 
> > Cc: sta...@vger.kernel.org
> > References: 
> > https://lists.freedesktop.org/archives/intel-gfx/2016-August/105545.html
> > References: 
> > https://lists.freedesktop.org/archives/dri-devel/2016-August/116888.html
> > References: 
> > https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html
> 
> References: 
> http://patchwork.freedesktop.org/patch/msgid/1473602239-15855-1-git-send-email-adrienve...@gmail.com
> 
> Acked-by: Jani Nikula 
> 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97060
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97443
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97363
> > Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
> > Tested-by: Marco Krüger 
> > Tested-by: Alexey Shumitsky 
> > Tested-by: Sean Greenslade 
> > Tested-by: Emil Andersen Lauridsen 
> > Tested-by: Robin Müller 
> > Tested-by: oceans...@gmail.com
> > Signed-off-by: Ville Syrjälä 

Slapped on another tested-by and pushed to dinq. Thanks for the broad
testing, everyone.

> > ---
> >  drivers/gpu/drm/i915/intel_opregion.c | 27 +++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> > b/drivers/gpu/drm/i915/intel_opregion.c
> > index adca262d591a..7acbbbf97833 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -1047,6 +1047,23 @@ err_out:
> > return err;
> >  }
> >  
> > +static int intel_use_opregion_panel_type_callback(const struct 
> > dmi_system_id *id)
> > +{
> > +   DRM_INFO("Using panel type from OpRegion on %s\n", id->ident);
> > +   return 1;
> > +}
> > +
> > +static const struct dmi_system_id intel_use_opregion_panel_type[] = {
> > +   {
> > +   .callback = intel_use_opregion_panel_type_callback,
> > +   .ident = "Conrac GmbH IX45GM2",
> > +   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Conrac GmbH"),
> > +   DMI_MATCH(DMI_PRODUCT_NAME, "IX45GM2"),
> > +   },
> > +   },
> > +   { }
> > +};
> > +
> >  int
> >  intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
> >  {
> > @@ -1073,6 +1090,16 @@ intel_opregion_get_panel_type(struct 
> > drm_i915_private *dev_priv)
> > }
> >  
> > /*
> > +* So far we know that some machined must use it, others must not use 
> > it.
> > +* There doesn't seem to be any way to determine which way to go, except
> > +* via a quirk list :(
> > +*/
> > +   if (!dmi_check_system(intel_use_opregion_panel_type)) {
> > +   DRM_DEBUG_KMS("Ignoring OpRegion panel type 

Re: [Intel-gfx] [PATCH] drm/i915: Ignore OpRegion panel type except on select machines

2016-09-13 Thread Jani Nikula
On Tue, 13 Sep 2016, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
>
> Turns out
> commit a05628195a0d ("drm/i915: Get panel_type from OpRegion panel
> details") has regressed quite a few machines. So it looks like we
> can't use the panel type from OpRegion on all systems, and yet we
> absolutely must use it on some specific systems.
>
> Despite trying, I was unable to find any automagic way to determine
> if the OpRegion panel type is respectable or not. The only glimmer
> of hope I had was bit 8 in the SCIC response, but that turned out to
> not work either (it was always 0 on both types of systems).
>
> So, to fix the regressions without breaking the machine we know to need
> the OpRegion panel type, let's just add a quirk for this. Only specific
> machines known to require the OpRegion panel type will therefore use
> it. Everyone else will fall bck to the VBT panel type.
>
> The only known machine so far is a "Conrac GmbH IX45GM2". The PCI
> subsystem ID on this machine is just a generic 8086:2a42, so of no use.
> Instead we'll go with a DMI match.
>
> I suspect we can now also revert
> commit aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")
> but let's leave that to a separate patch.
>
> v2: Do the DMI match in the opregion code directly, as dev_priv->quirks
> gets populated too late
>
> Cc: Rob Kramer 
> Cc: Martin van Es 
> Cc: Andrea Arcangeli 
> Cc: Dave Airlie 
> Cc: Marco Krüger 
> Cc: Sean Greenslade 
> Cc: Trudy Tective 
> Cc: Robin Müller 
> Cc: Alexander Kobel 
> Cc: Alexey Shumitsky 
> Cc: Emil Andersen Lauridsen 
> Cc: oceans...@gmail.com
> Cc: James Hogan 
> Cc: James Bottomley 
> Cc: sta...@vger.kernel.org
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2016-August/105545.html
> References: 
> https://lists.freedesktop.org/archives/dri-devel/2016-August/116888.html
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html

References: 
http://patchwork.freedesktop.org/patch/msgid/1473602239-15855-1-git-send-email-adrienve...@gmail.com

Acked-by: Jani Nikula 

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97060
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97443
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97363
> Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
> Tested-by: Marco Krüger 
> Tested-by: Alexey Shumitsky 
> Tested-by: Sean Greenslade 
> Tested-by: Emil Andersen Lauridsen 
> Tested-by: Robin Müller 
> Tested-by: oceans...@gmail.com
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index adca262d591a..7acbbbf97833 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -1047,6 +1047,23 @@ err_out:
>   return err;
>  }
>  
> +static int intel_use_opregion_panel_type_callback(const struct dmi_system_id 
> *id)
> +{
> + DRM_INFO("Using panel type from OpRegion on %s\n", id->ident);
> + return 1;
> +}
> +
> +static const struct dmi_system_id intel_use_opregion_panel_type[] = {
> + {
> + .callback = intel_use_opregion_panel_type_callback,
> + .ident = "Conrac GmbH IX45GM2",
> + .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Conrac GmbH"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "IX45GM2"),
> + },
> + },
> + { }
> +};
> +
>  int
>  intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
>  {
> @@ -1073,6 +1090,16 @@ intel_opregion_get_panel_type(struct drm_i915_private 
> *dev_priv)
>   }
>  
>   /*
> +  * So far we know that some machined must use it, others must not use 
> it.
> +  * There doesn't seem to be any way to determine which way to go, except
> +  * via a quirk list :(
> +  */
> + if (!dmi_check_system(intel_use_opregion_panel_type)) {
> + DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret - 1);
> + return -ENODEV;
> + }
> +
> + /*
>* FIXME On Dell XPS 13 9350 the OpRegion panel type (0) gives us
>* low vswing for eDP, whereas the VBT panel type (2) gives us normal
>* vswing instead. Low vswing results in some display flickers, so

-- 
Jani Nikula, Intel Open Source Technology 

[Intel-gfx] [PATCH] drm/i915: Ignore OpRegion panel type except on select machines

2016-09-13 Thread ville . syrjala
From: Ville Syrjälä 

Turns out
commit a05628195a0d ("drm/i915: Get panel_type from OpRegion panel
details") has regressed quite a few machines. So it looks like we
can't use the panel type from OpRegion on all systems, and yet we
absolutely must use it on some specific systems.

Despite trying, I was unable to find any automagic way to determine
if the OpRegion panel type is respectable or not. The only glimmer
of hope I had was bit 8 in the SCIC response, but that turned out to
not work either (it was always 0 on both types of systems).

So, to fix the regressions without breaking the machine we know to need
the OpRegion panel type, let's just add a quirk for this. Only specific
machines known to require the OpRegion panel type will therefore use
it. Everyone else will fall bck to the VBT panel type.

The only known machine so far is a "Conrac GmbH IX45GM2". The PCI
subsystem ID on this machine is just a generic 8086:2a42, so of no use.
Instead we'll go with a DMI match.

I suspect we can now also revert
commit aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")
but let's leave that to a separate patch.

v2: Do the DMI match in the opregion code directly, as dev_priv->quirks
gets populated too late

Cc: Rob Kramer 
Cc: Martin van Es 
Cc: Andrea Arcangeli 
Cc: Dave Airlie 
Cc: Marco Krüger 
Cc: Sean Greenslade 
Cc: Trudy Tective 
Cc: Robin Müller 
Cc: Alexander Kobel 
Cc: Alexey Shumitsky 
Cc: Emil Andersen Lauridsen 
Cc: oceans...@gmail.com
Cc: James Hogan 
Cc: James Bottomley 
Cc: sta...@vger.kernel.org
References: 
https://lists.freedesktop.org/archives/intel-gfx/2016-August/105545.html
References: 
https://lists.freedesktop.org/archives/dri-devel/2016-August/116888.html
References: 
https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97060
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97443
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97363
Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
Tested-by: Marco Krüger 
Tested-by: Alexey Shumitsky 
Tested-by: Sean Greenslade 
Tested-by: Emil Andersen Lauridsen 
Tested-by: Robin Müller 
Tested-by: oceans...@gmail.com
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_opregion.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index adca262d591a..7acbbbf97833 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -1047,6 +1047,23 @@ err_out:
return err;
 }
 
+static int intel_use_opregion_panel_type_callback(const struct dmi_system_id 
*id)
+{
+   DRM_INFO("Using panel type from OpRegion on %s\n", id->ident);
+   return 1;
+}
+
+static const struct dmi_system_id intel_use_opregion_panel_type[] = {
+   {
+   .callback = intel_use_opregion_panel_type_callback,
+   .ident = "Conrac GmbH IX45GM2",
+   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Conrac GmbH"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "IX45GM2"),
+   },
+   },
+   { }
+};
+
 int
 intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
 {
@@ -1073,6 +1090,16 @@ intel_opregion_get_panel_type(struct drm_i915_private 
*dev_priv)
}
 
/*
+* So far we know that some machined must use it, others must not use 
it.
+* There doesn't seem to be any way to determine which way to go, except
+* via a quirk list :(
+*/
+   if (!dmi_check_system(intel_use_opregion_panel_type)) {
+   DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret - 1);
+   return -ENODEV;
+   }
+
+   /*
 * FIXME On Dell XPS 13 9350 the OpRegion panel type (0) gives us
 * low vswing for eDP, whereas the VBT panel type (2) gives us normal
 * vswing instead. Low vswing results in some display flickers, so
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx