linux-kernel - Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Nov 2016 09:04:00 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        David Airlie <airlied@...ux.ie>,
        Archit Taneja <architt@...eaurora.org>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer
 hotplug handing to out of irq context

On Mon, Nov 28, 2016 at 10:46 PM, Laurent Pinchart
<laurent.pinchart@...asonboard.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@...ux.ie>
>> Cc: Archit Taneja <architt@...eaurora.org>
>> Cc: Wolfram Sang <wsa+renesas@...g-engineering.com>
>> Cc: Lars-Peter Clausen <lars@...afoo.de>
>> Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>> Cc: dri-devel@...ts.freedesktop.org
>> Signed-off-by: John Stultz <john.stultz@...aro.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@...asonboard.com>

Thanks so much for the review! I've made your suggested changes!
-john