On Wed, Dec 3, 2014 at 12:16 PM, jilaiw@codeaurora.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