linux-kernel - Re: [PATCH 2/2] drm/msm/hdmi: add hdmi hdcp support

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:	Wed, 3 Dec 2014 12:20:44 -0500
From:	Rob Clark <robdclark@...il.com>
To:	Jilai Wang <jilaiw@...eaurora.org>
Cc:	Bjorn Andersson <bjorn@...o.se>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	linux-arm-msm <linux-arm-msm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] drm/msm/hdmi: add hdmi hdcp support

On Wed, Dec 3, 2014 at 12:16 PM,  <jilaiw@...eaurora.org> wrote:
>>>> +       enum hdmi_hdcp_state hdcp_state;
>>>> +       struct mutex state_mutex;
>>>> +       struct delayed_work hdcp_reauth_work;
>>>> +       struct delayed_work hdcp_auth_part1_1_work;
>>>> +       struct delayed_work hdcp_auth_part1_2_work;
>>>> +       struct work_struct hdcp_auth_part1_3_work;
>>>> +       struct delayed_work hdcp_auth_part2_1_work;
>>>> +       struct delayed_work hdcp_auth_part2_2_work;
>>>> +       struct delayed_work hdcp_auth_part2_3_work;
>>>> +       struct delayed_work hdcp_auth_part2_4_work;
>>>> +       struct work_struct hdcp_auth_prepare_work;
>>>
>>> You shouldn't use "work" as a way to express states in your state
>>> machine.
>>> Better have 1 auth work function that does all these steps, probably
>>> having
>>> them split in functions just like you do now.
>>>
>>> That way you can have 1 function running the pass of authentication,
>>> starting
>>> by checking if you're reauthing or not then processing each step one by
>>> one,
>>> sleeping inbetween them. You can have the functions return -EAGAIN to
>>> indicate
>>> that you need to retry the current operation and so on.
>>>
>>> This would split the state machine from the state executioners and
>>> simplify
>>> your code.
>>
>> As a side note (disclaimer, I'm not an hdcp expert), but I wonder if
>> eventually some of that should be extracted out into some helpers in
>> drm core.  I guess that is something we'll figure out when a 2nd
>> driver gains upstream HDCP support.  One big work w/ msleep()'s does
>> sound like it would be easier to understand (and therefore easier to
>> refactor out into helpers).
>>
>> [snip]
>>
>
> The reason that I break the partI/PartII work into these small works
> because I want to avoid to use msleep in work.
> Otherwise cancel a HDCP work may cause long delay up to several seconds.


hmm, yeah, several seconds is long enough for user to plug/unplug
several displays ;-)

But I guess a wait_event_timeout() plus a wq that is signalled on hpd
(or whenever we need to cancel) instead of msleep might do the trick?

BR,
-R
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/