* [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set
@ 2013-10-13 11:56 Chris Wilson
2013-10-14 7:42 ` Jani Nikula
2013-10-14 8:03 ` Daniel Vetter
0 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2013-10-13 11:56 UTC (permalink / raw)
To: intel-gfx
At the moment we have 3 paths that lead to actually_set_backlight(),
from modesetting, ACPI/OpRegion requests and our very own
intel_backlight interface, and we have no way of distinguishing them in
the debug log. So add a debug breadcrumb to explain the source of the
backlight changes.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_opregion.c | 1 +
drivers/gpu/drm/i915/intel_panel.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 2acf5cae20e4..564da613b278 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -405,6 +405,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
if (bclp > 255)
return ASLC_BACKLIGHT_FAILED;
+ DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
intel_panel_set_backlight(dev, bclp, 255);
iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 54684168de1e..1f2996031ad9 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -574,6 +574,8 @@ void intel_panel_enable_backlight(struct drm_device *dev,
intel_pipe_to_cpu_transcoder(dev_priv, pipe);
unsigned long flags;
+ DRM_DEBUG_KMS("pipe=%d\n", pipe);
+
spin_lock_irqsave(&dev_priv->backlight.lock, flags);
if (dev_priv->backlight.level == 0) {
@@ -680,6 +682,8 @@ intel_panel_detect(struct drm_device *dev)
static int intel_panel_update_status(struct backlight_device *bd)
{
struct drm_device *dev = bl_get_data(bd);
+ DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
+ bd->props.brightness, bd->props.max_brightness);
intel_panel_set_backlight(dev, bd->props.brightness,
bd->props.max_brightness);
return 0;
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set
2013-10-13 11:56 [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set Chris Wilson
@ 2013-10-14 7:42 ` Jani Nikula
2013-10-14 7:54 ` Chris Wilson
2013-10-14 8:03 ` Daniel Vetter
1 sibling, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2013-10-14 7:42 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Sun, 13 Oct 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> At the moment we have 3 paths that lead to actually_set_backlight(),
> from modesetting, ACPI/OpRegion requests and our very own
> intel_backlight interface, and we have no way of distinguishing them in
> the debug log. So add a debug breadcrumb to explain the source of the
> backlight changes.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_opregion.c | 1 +
> drivers/gpu/drm/i915/intel_panel.c | 4 ++++
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 2acf5cae20e4..564da613b278 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -405,6 +405,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> if (bclp > 255)
> return ASLC_BACKLIGHT_FAILED;
>
> + DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
Just a few lines earliner we have:
DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
Other than that,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> intel_panel_set_backlight(dev, bclp, 255);
> iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 54684168de1e..1f2996031ad9 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -574,6 +574,8 @@ void intel_panel_enable_backlight(struct drm_device *dev,
> intel_pipe_to_cpu_transcoder(dev_priv, pipe);
> unsigned long flags;
>
> + DRM_DEBUG_KMS("pipe=%d\n", pipe);
> +
> spin_lock_irqsave(&dev_priv->backlight.lock, flags);
>
> if (dev_priv->backlight.level == 0) {
> @@ -680,6 +682,8 @@ intel_panel_detect(struct drm_device *dev)
> static int intel_panel_update_status(struct backlight_device *bd)
> {
> struct drm_device *dev = bl_get_data(bd);
> + DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
> + bd->props.brightness, bd->props.max_brightness);
> intel_panel_set_backlight(dev, bd->props.brightness,
> bd->props.max_brightness);
> return 0;
> --
> 1.8.4.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set
2013-10-14 7:42 ` Jani Nikula
@ 2013-10-14 7:54 ` Chris Wilson
2013-10-14 8:06 ` Jani Nikula
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2013-10-14 7:54 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Mon, Oct 14, 2013 at 10:42:37AM +0300, Jani Nikula wrote:
> On Sun, 13 Oct 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index 2acf5cae20e4..564da613b278 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -405,6 +405,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
> > if (bclp > 255)
> > return ASLC_BACKLIGHT_FAILED;
> >
> > + DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
>
> Just a few lines earliner we have:
>
> DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
Right, I wasn't sure about changing that to suite my purposes - I
guessed that it was intentionally a different debug level pre-validation
for a reason and so added a new message at the debug level as the
messages within the modesetting and backlight operation.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set
2013-10-13 11:56 [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set Chris Wilson
2013-10-14 7:42 ` Jani Nikula
@ 2013-10-14 8:03 ` Daniel Vetter
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2013-10-14 8:03 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Sun, Oct 13, 2013 at 12:56:31PM +0100, Chris Wilson wrote:
> At the moment we have 3 paths that lead to actually_set_backlight(),
> from modesetting, ACPI/OpRegion requests and our very own
> intel_backlight interface, and we have no way of distinguishing them in
> the debug log. So add a debug breadcrumb to explain the source of the
> backlight changes.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Yeah, we seem to have a bunch of bugs where the backlight somehow gets
smashed to 0 and we don't know why. Queued for -next, thanks for the
patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set
2013-10-14 7:54 ` Chris Wilson
@ 2013-10-14 8:06 ` Jani Nikula
2013-10-14 8:20 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2013-10-14 8:06 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Mon, 14 Oct 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Oct 14, 2013 at 10:42:37AM +0300, Jani Nikula wrote:
>> On Sun, 13 Oct 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>> > index 2acf5cae20e4..564da613b278 100644
>> > --- a/drivers/gpu/drm/i915/intel_opregion.c
>> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> > @@ -405,6 +405,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>> > if (bclp > 255)
>> > return ASLC_BACKLIGHT_FAILED;
>> >
>> > + DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
>>
>> Just a few lines earliner we have:
>>
>> DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
>
> Right, I wasn't sure about changing that to suite my purposes - I
> guessed that it was intentionally a different debug level pre-validation
> for a reason and so added a new message at the debug level as the
> messages within the modesetting and backlight operation.
Please just change that to suit your needs instead of adding a new one.
Jani.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set
2013-10-14 8:06 ` Jani Nikula
@ 2013-10-14 8:20 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2013-10-14 8:20 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Mon, Oct 14, 2013 at 10:06 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> On Mon, 14 Oct 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> On Mon, Oct 14, 2013 at 10:42:37AM +0300, Jani Nikula wrote:
>>> On Sun, 13 Oct 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>>> > index 2acf5cae20e4..564da613b278 100644
>>> > --- a/drivers/gpu/drm/i915/intel_opregion.c
>>> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
>>> > @@ -405,6 +405,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>>> > if (bclp > 255)
>>> > return ASLC_BACKLIGHT_FAILED;
>>> >
>>> > + DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
>>>
>>> Just a few lines earliner we have:
>>>
>>> DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
>>
>> Right, I wasn't sure about changing that to suite my purposes - I
>> guessed that it was intentionally a different debug level pre-validation
>> for a reason and so added a new message at the debug level as the
>> messages within the modesetting and backlight operation.
>
> Please just change that to suit your needs instead of adding a new one.
There's the validation of the bios request + masking in between these
two debug lines. So I think keeping them separate (one for the asle
backlight call and one for the actual set backlight request, if
there'll be one) seems ok. So I've merged the patch as -is.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-14 8:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-13 11:56 [PATCH] drm/i915: Add breadcrumbs for why the backlight is being set Chris Wilson
2013-10-14 7:42 ` Jani Nikula
2013-10-14 7:54 ` Chris Wilson
2013-10-14 8:06 ` Jani Nikula
2013-10-14 8:20 ` Daniel Vetter
2013-10-14 8:03 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox