* [PATCH] drm/i915: only report hpd connector status change when it actually changed
@ 2013-09-17 11:26 Jani Nikula
2013-09-17 14:36 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2013-09-17 11:26 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
This reduces dmesg noise when there's a glitch on the hpd line, or there
are more than one connectors on the same hpd line and only one of them
changes.
While at it, switch to use the friendly status names instead of numbers.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 13d26cf..a42f30b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -665,7 +665,8 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
crtc);
}
-static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *connector)
+static bool intel_hpd_irq_event(struct drm_device *dev,
+ struct drm_connector *connector)
{
enum drm_connector_status old_status;
@@ -673,11 +674,16 @@ static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *con
old_status = connector->status;
connector->status = connector->funcs->detect(connector, false);
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
+ if (old_status == connector->status)
+ return false;
+
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
connector->base.id,
drm_get_connector_name(connector),
- old_status, connector->status);
- return (old_status != connector->status);
+ drm_get_connector_status_name(old_status),
+ drm_get_connector_status_name(connector->status));
+
+ return true;
}
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: only report hpd connector status change when it actually changed
2013-09-17 11:26 [PATCH] drm/i915: only report hpd connector status change when it actually changed Jani Nikula
@ 2013-09-17 14:36 ` Daniel Vetter
2013-09-17 14:56 ` Jani Nikula
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2013-09-17 14:36 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Tue, Sep 17, 2013 at 02:26:34PM +0300, Jani Nikula wrote:
> This reduces dmesg noise when there's a glitch on the hpd line, or there
> are more than one connectors on the same hpd line and only one of them
> changes.
>
> While at it, switch to use the friendly status names instead of numbers.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 13d26cf..a42f30b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -665,7 +665,8 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
> crtc);
> }
>
> -static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *connector)
> +static bool intel_hpd_irq_event(struct drm_device *dev,
> + struct drm_connector *connector)
You change the return value to a bool here, but no users. Missing
follow-up patch?
-Daniel
> {
> enum drm_connector_status old_status;
>
> @@ -673,11 +674,16 @@ static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *con
> old_status = connector->status;
>
> connector->status = connector->funcs->detect(connector, false);
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
> + if (old_status == connector->status)
> + return false;
> +
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
> connector->base.id,
> drm_get_connector_name(connector),
> - old_status, connector->status);
> - return (old_status != connector->status);
> + drm_get_connector_status_name(old_status),
> + drm_get_connector_status_name(connector->status));
> +
> + return true;
> }
>
> /*
> --
> 1.7.9.5
>
> _______________________________________________
> 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] 4+ messages in thread
* Re: [PATCH] drm/i915: only report hpd connector status change when it actually changed
2013-09-17 14:36 ` Daniel Vetter
@ 2013-09-17 14:56 ` Jani Nikula
2013-09-17 15:27 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2013-09-17 14:56 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Tue, 17 Sep 2013, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Sep 17, 2013 at 02:26:34PM +0300, Jani Nikula wrote:
>> This reduces dmesg noise when there's a glitch on the hpd line, or there
>> are more than one connectors on the same hpd line and only one of them
>> changes.
>>
>> While at it, switch to use the friendly status names instead of numbers.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 13d26cf..a42f30b 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -665,7 +665,8 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
>> crtc);
>> }
>>
>> -static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *connector)
>> +static bool intel_hpd_irq_event(struct drm_device *dev,
>> + struct drm_connector *connector)
>
> You change the return value to a bool here, but no users. Missing
> follow-up patch?
Nope, the only call site in i915_hotplug_work_func() already treats it
as a bool.
Jani.
> -Daniel
>
>> {
>> enum drm_connector_status old_status;
>>
>> @@ -673,11 +674,16 @@ static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *con
>> old_status = connector->status;
>>
>> connector->status = connector->funcs->detect(connector, false);
>> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
>> + if (old_status == connector->status)
>> + return false;
>> +
>> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
>> connector->base.id,
>> drm_get_connector_name(connector),
>> - old_status, connector->status);
>> - return (old_status != connector->status);
>> + drm_get_connector_status_name(old_status),
>> + drm_get_connector_status_name(connector->status));
>> +
>> + return true;
>> }
>>
>> /*
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> 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] 4+ messages in thread
* Re: [PATCH] drm/i915: only report hpd connector status change when it actually changed
2013-09-17 14:56 ` Jani Nikula
@ 2013-09-17 15:27 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2013-09-17 15:27 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Tue, Sep 17, 2013 at 05:56:08PM +0300, Jani Nikula wrote:
> On Tue, 17 Sep 2013, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Sep 17, 2013 at 02:26:34PM +0300, Jani Nikula wrote:
> >> This reduces dmesg noise when there's a glitch on the hpd line, or there
> >> are more than one connectors on the same hpd line and only one of them
> >> changes.
> >>
> >> While at it, switch to use the friendly status names instead of numbers.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++----
> >> 1 file changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index 13d26cf..a42f30b 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -665,7 +665,8 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
> >> crtc);
> >> }
> >>
> >> -static int intel_hpd_irq_event(struct drm_device *dev, struct drm_connector *connector)
> >> +static bool intel_hpd_irq_event(struct drm_device *dev,
> >> + struct drm_connector *connector)
> >
> > You change the return value to a bool here, but no users. Missing
> > follow-up patch?
>
> Nope, the only call site in i915_hotplug_work_func() already treats it
> as a bool.
Somehow I've thought it started out with a void and you switched to bool.
Thanks for removing my blinders, patch merged ;-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-17 15:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 11:26 [PATCH] drm/i915: only report hpd connector status change when it actually changed Jani Nikula
2013-09-17 14:36 ` Daniel Vetter
2013-09-17 14:56 ` Jani Nikula
2013-09-17 15:27 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox