Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1531896 > unrolled thread
| Started by | John Stultz <john.stultz@linaro.org> |
|---|---|
| First post | 2016-11-29 06:10 +0100 |
| Last post | 2016-11-29 18:10 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context John Stultz <john.stultz@linaro.org> - 2016-11-29 06:10 +0100
Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-11-29 07:50 +0100
Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context John Stultz <john.stultz@linaro.org> - 2016-11-29 18:10 +0100
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-11-29 06:10 +0100 |
| Subject | [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context |
| Message-ID | <sIIQ1-3R4-5@gated-at.bofh.it> |
I was recently seeing issues with EDID probing, where
the logic to wait for the EDID read bit to be set by the
IRQ wasn't happening and the code would time out and fail.
Digging deeper, I found this was due to the fact that
IRQs were disabled as we were running in IRQ context from
the HPD signal.
Thus this patch changes the logic to handle the HPD signal
via a work_struct so we can be out of irq context.
With this patch, the EDID probing on hotplug does not time
out.
Cc: David Airlie <airlied@linux.ie>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Reworked to properly fix the issue rather then
just delaying for 200ms
drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 ++
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index 992d76c..2a1e722 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -317,6 +317,8 @@ struct adv7511 {
bool edid_read;
wait_queue_head_t wq;
+ struct work_struct irq_work;
+
struct drm_bridge bridge;
struct drm_connector connector;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8dba729..b38e743 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -402,6 +402,14 @@ static bool adv7511_hpd(struct adv7511 *adv7511)
return false;
}
+static void adv7511_irq_work(struct work_struct *work)
+{
+ struct adv7511 *adv7511 = container_of(work, struct adv7511, irq_work);
+
+ drm_helper_hpd_irq_event(adv7511->connector.dev);
+}
+
+
static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
{
unsigned int irq0, irq1;
@@ -419,7 +427,7 @@ static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1);
if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder)
- drm_helper_hpd_irq_event(adv7511->connector.dev);
+ schedule_work(&adv7511->irq_work);
if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) {
adv7511->edid_read = true;
@@ -1006,6 +1014,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
goto err_i2c_unregister_edid;
}
+ INIT_WORK(&adv7511->irq_work, adv7511_irq_work);
+
if (i2c->irq) {
init_waitqueue_head(&adv7511->wq);
--
2.7.4
[toc] | [next] | [standalone]
| From | Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
|---|---|
| Date | 2016-11-29 07:50 +0100 |
| Message-ID | <sIKoO-4Na-9@gated-at.bofh.it> |
| In reply to | #1531896 |
Hi John,
Thank you for the patch.
On Monday 28 Nov 2016 21:04:40 John Stultz wrote:
> I was recently seeing issues with EDID probing, where
> the logic to wait for the EDID read bit to be set by the
> IRQ wasn't happening and the code would time out and fail.
>
> Digging deeper, I found this was due to the fact that
> IRQs were disabled as we were running in IRQ context from
> the HPD signal.
>
> Thus this patch changes the logic to handle the HPD signal
> via a work_struct so we can be out of irq context.
>
> With this patch, the EDID probing on hotplug does not time
> out.
>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2: Reworked to properly fix the issue rather then
> just delaying for 200ms
>
> drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 ++
> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 992d76c..2a1e722 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -317,6 +317,8 @@ struct adv7511 {
> bool edid_read;
>
> wait_queue_head_t wq;
> + struct work_struct irq_work;
> +
I'd call this hpd_work.
> struct drm_bridge bridge;
> struct drm_connector connector;
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b38e743
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -402,6 +402,14 @@ static bool adv7511_hpd(struct adv7511 *adv7511)
> return false;
> }
>
> +static void adv7511_irq_work(struct work_struct *work)
> +{
> + struct adv7511 *adv7511 = container_of(work, struct adv7511,
irq_work);
> +
> + drm_helper_hpd_irq_event(adv7511->connector.dev);
> +}
> +
> +
One blank line is enough.
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
> {
> unsigned int irq0, irq1;
> @@ -419,7 +427,7 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
> bool process_hpd) regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1);
>
> if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder)
> - drm_helper_hpd_irq_event(adv7511->connector.dev);
> + schedule_work(&adv7511->irq_work);
>
> if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) {
> adv7511->edid_read = true;
> @@ -1006,6 +1014,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) goto err_i2c_unregister_edid;
> }
>
> + INIT_WORK(&adv7511->irq_work, adv7511_irq_work);
> +
> if (i2c->irq) {
> init_waitqueue_head(&adv7511->wq);
--
Regards,
Laurent Pinchart
[toc] | [prev] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-11-29 18:10 +0100 |
| Subject | Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context |
| Message-ID | <sIU4N-2TK-25@gated-at.bofh.it> |
| In reply to | #1531938 |
On Mon, Nov 28, 2016 at 10:46 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi John,
>
> Thank you for the patch.
>
> On Monday 28 Nov 2016 21:04:40 John Stultz wrote:
>> I was recently seeing issues with EDID probing, where
>> the logic to wait for the EDID read bit to be set by the
>> IRQ wasn't happening and the code would time out and fail.
>>
>> Digging deeper, I found this was due to the fact that
>> IRQs were disabled as we were running in IRQ context from
>> the HPD signal.
>>
>> Thus this patch changes the logic to handle the HPD signal
>> via a work_struct so we can be out of irq context.
>>
>> With this patch, the EDID probing on hotplug does not time
>> out.
>>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Archit Taneja <architt@codeaurora.org>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: Lars-Peter Clausen <lars@metafoo.de>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>> v2: Reworked to properly fix the issue rather then
>> just delaying for 200ms
>>
>> drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 ++
>> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++++++++++-
>> 2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 992d76c..2a1e722 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> @@ -317,6 +317,8 @@ struct adv7511 {
>> bool edid_read;
>>
>> wait_queue_head_t wq;
>> + struct work_struct irq_work;
>> +
>
> I'd call this hpd_work.
>
>> struct drm_bridge bridge;
>> struct drm_connector connector;
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b38e743
>> 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -402,6 +402,14 @@ static bool adv7511_hpd(struct adv7511 *adv7511)
>> return false;
>> }
>>
>> +static void adv7511_irq_work(struct work_struct *work)
>> +{
>> + struct adv7511 *adv7511 = container_of(work, struct adv7511,
> irq_work);
>> +
>> + drm_helper_hpd_irq_event(adv7511->connector.dev);
>> +}
>> +
>> +
>
> One blank line is enough.
>
> Apart from that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Thanks so much for the review! I've made your suggested changes!
-john
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web