Re: [PATCH v3 05/40] misc/mei/hdcp: Notifier chain for mei cldev state change

[Ramalingam C <ramalingam.c@intel.com>]

On Thursday 17 May 2018 11:22 AM, Usyskin, Alexander wrote:
>
>> -----Original Message-----
>> From: C, Ramalingam
>> Sent: Wednesday, May 16, 2018 16:05
>> To: Usyskin, Alexander <alexander.usyskin@intel.com>; intel-
>> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>> seanpaul@chromium.org; daniel@ffwll.ch; chris@chris-wilson.co.uk;
>> jani.nikula@linux.intel.com; Winkler, Tomas <tomas.winkler@intel.com>
>> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Subject: Re: [PATCH v3 05/40] misc/mei/hdcp: Notifier chain for mei cldev
>> state change
>>
>> Thanks Alexander for the review. Sorry for the delay in addressing the review
>> comments
>>
>>
>> On Wednesday 04 April 2018 11:42 AM, Usyskin, Alexander wrote:
>>>> -----Original Message-----
>>>> From: C, Ramalingam
>>>> Sent: Tuesday, April 03, 2018 16:57
>>>> To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>>>> seanpaul@chromium.org; daniel@ffwll.ch; chris@chris-wilson.co.uk;
>>>> jani.nikula@linux.intel.com; Winkler, Tomas <tomas.winkler@intel.com>;
>>>> Usyskin, Alexander <alexander.usyskin@intel.com>
>>>> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; C, Ramalingam
>>>> <ramalingam.c@intel.com>
>>>> Subject: [PATCH v3 05/40] misc/mei/hdcp: Notifier chain for mei cldev
>> state
>>>> change
>>>>
>>>> Notifier Chain is defined to inform all its clients about the mei
>>>> client device state change. Routine is defined for the clients to
>>>> register and unregister for the notification on state change.
>>>>
>>>> v2:
>>>>     Rebased.
>>>> v3:
>>>>     Notifier chain is adopted for cldev state update [Tomas]
>>>>
>>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>>> ---
>>> May be I miss something, but bus subsystem already has notifier chain.
>> (BUS_NOTIFY_ADD_DEVICE etc.)
>>> We only ought to make mei_cl_bus_type available in header.
>> We need re notification incase of binding of the mei device happened
>> well before the I915 init.
>> This is the case if I915 and MEI HDCP are build as modules and loaded in
>> order.
>>
>> So I prefer the separate notifier for mei_hdcp. Hope that is fine.
>>
>> --Ram
> Still miss the part how you design coupe with mei device loaded well before i915?
> Can you explain?

If you build both I915 and mei_hdcp as modules, as mei_hdcp provides 
some symbols for the I915, mei_hdcp needs to be loaded first.
Hence mei hdcp client device binding is completed before the I915 Load. 
So notifier triggered even before its listener I915 is ready.
So When you load the I915 after mei hdcp client device binding, We need 
to trigger the notifier calls once again, so that I915 will get the 
handle cldev.


--Ram
>
>>>>    drivers/misc/mei/hdcp/mei_hdcp.c | 36
>> +++++++++++++++++++++++++++---
>>>>    include/linux/mei_hdcp.h         | 48
>>>> ++++++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 81 insertions(+), 3 deletions(-)
>>>>    create mode 100644 include/linux/mei_hdcp.h
>>>>
>>>> diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
>>>> b/drivers/misc/mei/hdcp/mei_hdcp.c
>>>> index b334615728a7..2811a25f8c57 100644
>>>> --- a/drivers/misc/mei/hdcp/mei_hdcp.c
>>>> +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
>>>> @@ -31,6 +31,32 @@
>>>>    #include <linux/slab.h>
>>>>    #include <linux/uuid.h>
>>>>    #include <linux/mei_cl_bus.h>
>>>> +#include <linux/notifier.h>
>>>> +#include <linux/mei_hdcp.h>
>>>> +
>>>> +static BLOCKING_NOTIFIER_HEAD(mei_cldev_notifier_list);
>>>> +
>>>> +void mei_cldev_state_notify_clients(struct mei_cl_device *cldev, bool
>>>> enabled)
>>>> +{
>>>> +	if (enabled)
>>>> +		blocking_notifier_call_chain(&mei_cldev_notifier_list,
>>>> +					     MEI_CLDEV_ENABLED, cldev);
>>>> +	else
>>>> +		blocking_notifier_call_chain(&mei_cldev_notifier_list,
>>>> +					     MEI_CLDEV_DISABLED, NULL);
>>>> +}
>>>> +
>>>> +int mei_cldev_register_notify(struct notifier_block *nb)
>>>> +{
>>>> +	return blocking_notifier_chain_register(&mei_cldev_notifier_list,
>>>> nb);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(mei_cldev_register_notify);
>>>> +
>>>> +int mei_cldev_unregister_notify(struct notifier_block *nb)
>>>> +{
>>>> +	return blocking_notifier_chain_unregister(&mei_cldev_notifier_list,
>>>> nb);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(mei_cldev_unregister_notify);
>>>>
>>>>    static int mei_hdcp_probe(struct mei_cl_device *cldev,
>>>>    			  const struct mei_cl_device_id *id)
>>>> @@ -38,14 +64,18 @@ static int mei_hdcp_probe(struct mei_cl_device
>>>> *cldev,
>>>>    	int ret;
>>>>
>>>>    	ret = mei_cldev_enable(cldev);
>>>> -	if (ret < 0)
>>>> +	if (ret < 0) {
>>>>    		dev_err(&cldev->dev, "mei_cldev_enable Failed. %d\n", ret);
>>>> +		return ret;
>>>> +	}
>>>>
>>>> -	return ret;
>>>> +	mei_cldev_state_notify_clients(cldev, true);
>>>> +	return 0;
>>>>    }
>>>>
>>>>    static int mei_hdcp_remove(struct mei_cl_device *cldev)
>>>>    {
>>>> +	mei_cldev_state_notify_clients(cldev, false);
>>>>    	mei_cldev_set_drvdata(cldev, NULL);
>>>>    	return mei_cldev_disable(cldev);
>>>>    }
>>>> @@ -71,4 +101,4 @@ module_mei_cl_driver(mei_hdcp_driver);
>>>>
>>>>    MODULE_AUTHOR("Intel Corporation");
>>>>    MODULE_LICENSE("Dual BSD/GPL");
>>>> -MODULE_DESCRIPTION("HDCP");
>>>> +MODULE_DESCRIPTION("MEI HDCP");
>>>> diff --git a/include/linux/mei_hdcp.h b/include/linux/mei_hdcp.h
>>>> new file mode 100644
>>>> index 000000000000..3b46bebde718
>>>> --- /dev/null
>>>> +++ b/include/linux/mei_hdcp.h
>>>> @@ -0,0 +1,48 @@
>>>> +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
>>>> +/*
>>>> + * Copyright © 2017-2018 Intel Corporation
>>>> + *
>>>> + * Permission to use, copy, modify, distribute, and sell this software and
>> its
>>>> + * documentation for any purpose is hereby granted without fee,
>> provided
>>>> that
>>>> + * the above copyright notice appear in all copies and that both that
>>>> copyright
>>>> + * notice and this permission notice appear in supporting
>> documentation,
>>>> and
>>>> + * that the name of the copyright holders not be used in advertising or
>>>> + * publicity pertaining to distribution of the software without specific,
>>>> + * written prior permission.  The copyright holders make no
>> representations
>>>> + * about the suitability of this software for any purpose.  It is provided
>> "as
>>>> + * is" without express or implied warranty.
>>>> + *
>>>> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD
>> TO
>>>> THIS SOFTWARE,
>>>> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
>>>> FITNESS, IN NO
>>>> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL,
>>>> INDIRECT OR
>>>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
>> RESULTING
>>>> FROM LOSS OF USE,
>>>> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
>> NEGLIGENCE
>>>> OR OTHER
>>>> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE
>> USE
>>>> OR PERFORMANCE
>>>> + * OF THIS SOFTWARE.
>>>> + *
>>>> + * Authors:
>>>> + *	Ramalingam C <ramalingam.c@intel.com>
>>>> + */
>>>> +
>>>> +#ifndef _LINUX_MEI_HDCP_H
>>>> +#define _LINUX_MEI_HDCP_H
>>>> +
>>>> +enum mei_cldev_state {
>>>> +	MEI_CLDEV_DISABLED,
>>>> +	MEI_CLDEV_ENABLED
>>>> +};
>>>> +
>>>> +#ifdef CONFIG_INTEL_MEI_HDCP
>>>> +int mei_cldev_register_notify(struct notifier_block *nb);
>>>> +int mei_cldev_unregister_notify(struct notifier_block *nb);
>>>> +#else
>>>> +static int mei_cldev_register_notify(struct notifier_block *nb)
>>>> +{
>>>> +	return -ENODEV;
>>>> +}
>>>> +static int mei_cldev_unregister_notify(struct notifier_block *nb)
>>>> +{
>>>> +	return -ENODEV;
>>>> +}
>>>> +#endif /* defined (CONFIG_INTEL_MEI_HDCP) */
>>>> +#endif /* defined (_LINUX_MEI_HDCP_H) */
>>>> --
>>>> 2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel