public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present
@ 2014-01-16 16:27 ville.syrjala
  2014-01-16 16:55 ` Daniel Vetter
  2014-01-17 11:30 ` Jani Nikula
  0 siblings, 2 replies; 5+ messages in thread
From: ville.syrjala @ 2014-01-16 16:27 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My 855gm doesn't register the intel backlight but it still ends up
calling the backlight code to enable/disable the backlight via the
LVDS code. This leads to some WARNs due to backlight.max being 0.

Let's have intel_panel_enable_backlight() and intel_panel_disable_backlight()
check whether there's a backlight present or not.

Also move the backlight.present check from asle_set_backlight() into
intel_panel_set_backlight() for some extra symmetry.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_opregion.c | 10 ++--------
 drivers/gpu/drm/i915/intel_panel.c    |  6 +++---
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 3da259e..37e9a96 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -396,9 +396,7 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_connector *connector;
 	struct intel_connector *intel_connector;
-	struct intel_panel *panel;
 	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
 
 	DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
@@ -417,12 +415,8 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 	 * only one).
 	 */
 	DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		intel_connector = to_intel_connector(connector);
-		panel = &intel_connector->panel;
-		if (panel->backlight.present)
-			intel_panel_set_backlight(intel_connector, bclp, 255);
-	}
+	list_for_each_entry(intel_connector, &dev->mode_config.connector_list, base.head)
+		intel_panel_set_backlight(intel_connector, bclp, 255);
 	iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
 
 	mutex_unlock(&dev->mode_config.mutex);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 20ebc3e..350de35 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -502,7 +502,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
 	u32 freq;
 	unsigned long flags;
 
-	if (pipe == INVALID_PIPE)
+	if (!panel->backlight.present || pipe == INVALID_PIPE)
 		return;
 
 	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
@@ -579,7 +579,7 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
 	enum pipe pipe = intel_get_pipe_from_connector(connector);
 	unsigned long flags;
 
-	if (pipe == INVALID_PIPE)
+	if (!panel->backlight.present || pipe == INVALID_PIPE)
 		return;
 
 	/*
@@ -782,7 +782,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
 	enum pipe pipe = intel_get_pipe_from_connector(connector);
 	unsigned long flags;
 
-	if (pipe == INVALID_PIPE)
+	if (!panel->backlight.present || pipe == INVALID_PIPE)
 		return;
 
 	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
-- 
1.8.3.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present
  2014-01-16 16:27 [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present ville.syrjala
@ 2014-01-16 16:55 ` Daniel Vetter
  2014-01-16 17:09   ` Ville Syrjälä
  2014-01-17 11:30 ` Jani Nikula
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2014-01-16 16:55 UTC (permalink / raw)
  To: Syrjala, Ville; +Cc: intel-gfx

On Thu, Jan 16, 2014 at 5:27 PM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My 855gm doesn't register the intel backlight but it still ends up
> calling the backlight code to enable/disable the backlight via the
> LVDS code. This leads to some WARNs due to backlight.max being 0.
>
> Let's have intel_panel_enable_backlight() and intel_panel_disable_backlight()
> check whether there's a backlight present or not.
>
> Also move the backlight.present check from asle_set_backlight() into
> intel_panel_set_backlight() for some extra symmetry.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Wouldn't that be a perfect machine to test Jani's
init-backlight-from-vbt patch? Iirc all the infrastructure is now
there ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present
  2014-01-16 16:55 ` Daniel Vetter
@ 2014-01-16 17:09   ` Ville Syrjälä
  0 siblings, 0 replies; 5+ messages in thread
From: Ville Syrjälä @ 2014-01-16 17:09 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Thu, Jan 16, 2014 at 05:55:48PM +0100, Daniel Vetter wrote:
> On Thu, Jan 16, 2014 at 5:27 PM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > My 855gm doesn't register the intel backlight but it still ends up
> > calling the backlight code to enable/disable the backlight via the
> > LVDS code. This leads to some WARNs due to backlight.max being 0.
> >
> > Let's have intel_panel_enable_backlight() and intel_panel_disable_backlight()
> > check whether there's a backlight present or not.
> >
> > Also move the backlight.present check from asle_set_backlight() into
> > intel_panel_set_backlight() for some extra symmetry.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Wouldn't that be a perfect machine to test Jani's
> init-backlight-from-vbt patch? Iirc all the infrastructure is now
> there ...

My assumption has ben that the intel backlight stuff isn't hooked up
to anything. But I guess I could give it a go.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present
  2014-01-16 16:27 [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present ville.syrjala
  2014-01-16 16:55 ` Daniel Vetter
@ 2014-01-17 11:30 ` Jani Nikula
  2014-01-17 13:32   ` Daniel Vetter
  1 sibling, 1 reply; 5+ messages in thread
From: Jani Nikula @ 2014-01-17 11:30 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Thu, 16 Jan 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My 855gm doesn't register the intel backlight but it still ends up
> calling the backlight code to enable/disable the backlight via the
> LVDS code. This leads to some WARNs due to backlight.max being 0.
>
> Let's have intel_panel_enable_backlight() and intel_panel_disable_backlight()
> check whether there's a backlight present or not.
>
> Also move the backlight.present check from asle_set_backlight() into
> intel_panel_set_backlight() for some extra symmetry.

I have an identical patch in my local repo dated Dec 13... should've
sent it out weeks ago. Damn.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 10 ++--------
>  drivers/gpu/drm/i915/intel_panel.c    |  6 +++---
>  2 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 3da259e..37e9a96 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -396,9 +396,7 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
>  static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_connector *connector;
>  	struct intel_connector *intel_connector;
> -	struct intel_panel *panel;
>  	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
>  
>  	DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
> @@ -417,12 +415,8 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>  	 * only one).
>  	 */
>  	DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
> -	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> -		intel_connector = to_intel_connector(connector);
> -		panel = &intel_connector->panel;
> -		if (panel->backlight.present)
> -			intel_panel_set_backlight(intel_connector, bclp, 255);
> -	}
> +	list_for_each_entry(intel_connector, &dev->mode_config.connector_list, base.head)
> +		intel_panel_set_backlight(intel_connector, bclp, 255);
>  	iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
>  
>  	mutex_unlock(&dev->mode_config.mutex);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 20ebc3e..350de35 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -502,7 +502,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
>  	u32 freq;
>  	unsigned long flags;
>  
> -	if (pipe == INVALID_PIPE)
> +	if (!panel->backlight.present || pipe == INVALID_PIPE)
>  		return;
>  
>  	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
> @@ -579,7 +579,7 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
>  	enum pipe pipe = intel_get_pipe_from_connector(connector);
>  	unsigned long flags;
>  
> -	if (pipe == INVALID_PIPE)
> +	if (!panel->backlight.present || pipe == INVALID_PIPE)
>  		return;
>  
>  	/*
> @@ -782,7 +782,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>  	enum pipe pipe = intel_get_pipe_from_connector(connector);
>  	unsigned long flags;
>  
> -	if (pipe == INVALID_PIPE)
> +	if (!panel->backlight.present || pipe == INVALID_PIPE)
>  		return;
>  
>  	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
> -- 
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present
  2014-01-17 11:30 ` Jani Nikula
@ 2014-01-17 13:32   ` Daniel Vetter
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2014-01-17 13:32 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Jan 17, 2014 at 01:30:59PM +0200, Jani Nikula wrote:
> On Thu, 16 Jan 2014, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > My 855gm doesn't register the intel backlight but it still ends up
> > calling the backlight code to enable/disable the backlight via the
> > LVDS code. This leads to some WARNs due to backlight.max being 0.
> >
> > Let's have intel_panel_enable_backlight() and intel_panel_disable_backlight()
> > check whether there's a backlight present or not.
> >
> > Also move the backlight.present check from asle_set_backlight() into
> > intel_panel_set_backlight() for some extra symmetry.
> 
> I have an identical patch in my local repo dated Dec 13... should've
> sent it out weeks ago. Damn.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_opregion.c | 10 ++--------
> >  drivers/gpu/drm/i915/intel_panel.c    |  6 +++---
> >  2 files changed, 5 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index 3da259e..37e9a96 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -396,9 +396,7 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
> >  static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct drm_connector *connector;
> >  	struct intel_connector *intel_connector;
> > -	struct intel_panel *panel;
> >  	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
> >  
> >  	DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
> > @@ -417,12 +415,8 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> >  	 * only one).
> >  	 */
> >  	DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
> > -	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> > -		intel_connector = to_intel_connector(connector);
> > -		panel = &intel_connector->panel;
> > -		if (panel->backlight.present)
> > -			intel_panel_set_backlight(intel_connector, bclp, 255);
> > -	}
> > +	list_for_each_entry(intel_connector, &dev->mode_config.connector_list, base.head)
> > +		intel_panel_set_backlight(intel_connector, bclp, 255);
> >  	iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
> >  
> >  	mutex_unlock(&dev->mode_config.mutex);
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 20ebc3e..350de35 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -502,7 +502,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
> >  	u32 freq;
> >  	unsigned long flags;
> >  
> > -	if (pipe == INVALID_PIPE)
> > +	if (!panel->backlight.present || pipe == INVALID_PIPE)
> >  		return;
> >  
> >  	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
> > @@ -579,7 +579,7 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
> >  	enum pipe pipe = intel_get_pipe_from_connector(connector);
> >  	unsigned long flags;
> >  
> > -	if (pipe == INVALID_PIPE)
> > +	if (!panel->backlight.present || pipe == INVALID_PIPE)
> >  		return;
> >  
> >  	/*
> > @@ -782,7 +782,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
> >  	enum pipe pipe = intel_get_pipe_from_connector(connector);
> >  	unsigned long flags;
> >  
> > -	if (pipe == INVALID_PIPE)
> > +	if (!panel->backlight.present || pipe == INVALID_PIPE)
> >  		return;
> >  
> >  	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
> > -- 
> > 1.8.3.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-01-17 13:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-16 16:27 [PATCH] drm/i915: Eliminate lots of WARNs when there's no backlight present ville.syrjala
2014-01-16 16:55 ` Daniel Vetter
2014-01-16 17:09   ` Ville Syrjälä
2014-01-17 11:30 ` Jani Nikula
2014-01-17 13:32   ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox