diff mbox series

[V2,1/4] firmware: arm_scmi: Add perf_notify_support interface

Message ID 20240117104116.2055349-2-quic_sibis@quicinc.com
State New
Headers
Series firmware: arm_scmi: Register and handle limits change notification |

Commit Message

Sibi Sankar Jan. 17, 2024, 10:41 a.m. UTC 
  Add a new perf_notify_support interface to the existing perf_ops to export
info regarding limit/level change notification support.

Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 drivers/firmware/arm_scmi/perf.c | 16 ++++++++++++++++
 include/linux/scmi_protocol.h    |  8 ++++++++
 2 files changed, 24 insertions(+)

Comments

Cristian Marussi Jan. 29, 2024, 3:50 p.m. UTC | #1 
On Wed, Jan 17, 2024 at 04:11:13PM +0530, Sibi Sankar wrote:
> Add a new perf_notify_support interface to the existing perf_ops to export
> info regarding limit/level change notification support.
> 

Hi Sibi,

as I mentioned previously, in order not to add a needless stream of SCMI
Perf accessors I posted this:

https://lore.kernel.org/linux-arm-kernel/20240129151002.1215333-1-cristian.marussi@arm.com/T/#u

to expose all the Perf domains infos via the usual info_get(), similarly
to how other SCMI protocols do already.

I think that reworking this series on that, you can certainly drop this patch and just
check the _notify booleans on the retrieved domain info.

Thanks,
Cristian
Cristian Marussi Jan. 29, 2024, 5:33 p.m. UTC | #2 
On Mon, Jan 29, 2024 at 03:50:20PM +0000, Cristian Marussi wrote:
> On Wed, Jan 17, 2024 at 04:11:13PM +0530, Sibi Sankar wrote:
> > Add a new perf_notify_support interface to the existing perf_ops to export
> > info regarding limit/level change notification support.
> > 
> 
> Hi Sibi,
> 
> as I mentioned previously, in order not to add a needless stream of SCMI
> Perf accessors I posted this:
> 
> https://lore.kernel.org/linux-arm-kernel/20240129151002.1215333-1-cristian.marussi@arm.com/T/#u
> 
> to expose all the Perf domains infos via the usual info_get(), similarly
> to how other SCMI protocols do already.
> 
> I think that reworking this series on that, you can certainly drop this patch and just
> check the _notify booleans on the retrieved domain info.

Sorry, but hold on with this change, I will probably post an updated version
my patch above.

Thanks,
Cristian
Sudeep Holla Jan. 31, 2024, 11:28 a.m. UTC | #3 
On Mon, Jan 29, 2024 at 05:33:42PM +0000, Cristian Marussi wrote:
> On Mon, Jan 29, 2024 at 03:50:20PM +0000, Cristian Marussi wrote:
> > On Wed, Jan 17, 2024 at 04:11:13PM +0530, Sibi Sankar wrote:
> > > Add a new perf_notify_support interface to the existing perf_ops to export
> > > info regarding limit/level change notification support.
> > > 
> > 
> > Hi Sibi,
> > 
> > as I mentioned previously, in order not to add a needless stream of SCMI
> > Perf accessors I posted this:
> > 
> > https://lore.kernel.org/linux-arm-kernel/20240129151002.1215333-1-cristian.marussi@arm.com/T/#u
> > 
> > to expose all the Perf domains infos via the usual info_get(), similarly
> > to how other SCMI protocols do already.
> > 
> > I think that reworking this series on that, you can certainly drop this patch and just
> > check the _notify booleans on the retrieved domain info.
> 
> Sorry, but hold on with this change, I will probably post an updated version
> my patch above.
> 

As discussed in private, I would prefer to avoid exposing all the internals
to the users of SCMI perf. At the same time may we can do better if we can
check the availability of notification as part of notification enablement
from the SCMI driver, I need to think the details yet.
Cristian Marussi Jan. 31, 2024, 11:35 a.m. UTC | #4 
On Wed, Jan 31, 2024 at 11:28:54AM +0000, Sudeep Holla wrote:
> On Mon, Jan 29, 2024 at 05:33:42PM +0000, Cristian Marussi wrote:
> > On Mon, Jan 29, 2024 at 03:50:20PM +0000, Cristian Marussi wrote:
> > > On Wed, Jan 17, 2024 at 04:11:13PM +0530, Sibi Sankar wrote:
> > > > Add a new perf_notify_support interface to the existing perf_ops to export
> > > > info regarding limit/level change notification support.
> > > > 
> > > 
> > > Hi Sibi,
> > > 
> > > as I mentioned previously, in order not to add a needless stream of SCMI
> > > Perf accessors I posted this:
> > > 
> > > https://lore.kernel.org/linux-arm-kernel/20240129151002.1215333-1-cristian.marussi@arm.com/T/#u
> > > 
> > > to expose all the Perf domains infos via the usual info_get(), similarly
> > > to how other SCMI protocols do already.
> > > 
> > > I think that reworking this series on that, you can certainly drop this patch and just
> > > check the _notify booleans on the retrieved domain info.
> > 
> > Sorry, but hold on with this change, I will probably post an updated version
> > my patch above.
> > 
> 
> As discussed in private, I would prefer to avoid exposing all the internals
> to the users of SCMI perf. At the same time may we can do better if we can
> check the availability of notification as part of notification enablement
> from the SCMI driver, I need to think the details yet.

Yes a patch is under-work to avoid exposing too much Perf info AND to
avoid adding ad-hoc accessors like xlate, in this case.

Thanks,
Cristian
Cristian Marussi Feb. 12, 2024, 12:44 p.m. UTC | #5 
On Wed, Jan 31, 2024 at 11:28:54AM +0000, Sudeep Holla wrote:
> On Mon, Jan 29, 2024 at 05:33:42PM +0000, Cristian Marussi wrote:

Hi Sibi,

> > On Mon, Jan 29, 2024 at 03:50:20PM +0000, Cristian Marussi wrote:
> > > On Wed, Jan 17, 2024 at 04:11:13PM +0530, Sibi Sankar wrote:
> > > > Add a new perf_notify_support interface to the existing perf_ops to export
> > > > info regarding limit/level change notification support.
> > > > 
> > > 
> > > Hi Sibi,
> > > 
> > > as I mentioned previously, in order not to add a needless stream of SCMI
> > > Perf accessors I posted this:
> > > 
> > > https://lore.kernel.org/linux-arm-kernel/20240129151002.1215333-1-cristian.marussi@arm.com/T/#u
> > > 
> > > to expose all the Perf domains infos via the usual info_get(), similarly
> > > to how other SCMI protocols do already.
> > > 
> > > I think that reworking this series on that, you can certainly drop this patch and just
> > > check the _notify booleans on the retrieved domain info.
> > 
> > Sorry, but hold on with this change, I will probably post an updated version
> > my patch above.
> > 
> 
> As discussed in private, I would prefer to avoid exposing all the internals
> to the users of SCMI perf. At the same time may we can do better if we can
> check the availability of notification as part of notification enablement
> from the SCMI driver, I need to think the details yet.
> 

as previously mentioned, after speaking with Sudeep, I posted a new
series at [1], that aims to solve your problem with registering
notifications and looking up reported Perf frequencies in a new way.

Using the changes at [1] you should be able to:

- register your notifier without caring to check if the notification
  is supported, since the SCMI core will take care of checking that and
  report an error if not supported, without sending any unneeded
  attempted notification enable message  (so you can drop 1/4 in this
  series)

- retrieve the pre-calculated OPPs frequencies from the new extended
  Perf notifications reports no matter if the system if operating in
  level_indexing_mode or not. (so you can drop 2/4 in this series)

Thanks,
Cristian

[1]: https://lore.kernel.org/linux-arm-kernel/20240212123233.1230090-1-cristian.marussi@arm.com/T/#ma68cefd753e34ba3e1f2d4392e978026a87e1bf8
Sibi Sankar Feb. 13, 2024, 5:40 a.m. UTC | #6 
On 2/12/24 18:14, Cristian Marussi wrote:
> On Wed, Jan 31, 2024 at 11:28:54AM +0000, Sudeep Holla wrote:
>> On Mon, Jan 29, 2024 at 05:33:42PM +0000, Cristian Marussi wrote:
> 
> Hi Sibi,
> 
>>> On Mon, Jan 29, 2024 at 03:50:20PM +0000, Cristian Marussi wrote:
>>>> On Wed, Jan 17, 2024 at 04:11:13PM +0530, Sibi Sankar wrote:
>>>>> Add a new perf_notify_support interface to the existing perf_ops to export
>>>>> info regarding limit/level change notification support.
>>>>>
>>>>
>>>> Hi Sibi,
>>>>
>>>> as I mentioned previously, in order not to add a needless stream of SCMI
>>>> Perf accessors I posted this:
>>>>
>>>> https://lore.kernel.org/linux-arm-kernel/20240129151002.1215333-1-cristian.marussi@arm.com/T/#u
>>>>
>>>> to expose all the Perf domains infos via the usual info_get(), similarly
>>>> to how other SCMI protocols do already.
>>>>
>>>> I think that reworking this series on that, you can certainly drop this patch and just
>>>> check the _notify booleans on the retrieved domain info.
>>>
>>> Sorry, but hold on with this change, I will probably post an updated version
>>> my patch above.
>>>
>>
>> As discussed in private, I would prefer to avoid exposing all the internals
>> to the users of SCMI perf. At the same time may we can do better if we can
>> check the availability of notification as part of notification enablement
>> from the SCMI driver, I need to think the details yet.
>>
> 
> as previously mentioned, after speaking with Sudeep, I posted a new
> series at [1], that aims to solve your problem with registering
> notifications and looking up reported Perf frequencies in a new way.
> 
> Using the changes at [1] you should be able to:
> 
> - register your notifier without caring to check if the notification
>    is supported, since the SCMI core will take care of checking that and
>    report an error if not supported, without sending any unneeded
>    attempted notification enable message  (so you can drop 1/4 in this
>    series)
> 
> - retrieve the pre-calculated OPPs frequencies from the new extended
>    Perf notifications reports no matter if the system if operating in
>    level_indexing_mode or not. (so you can drop 2/4 in this series)

Christian/Sudeep,

Thanks a lot for spending time on this and simplifying the series.

Will re-spin the series with your recommendations.

-Sibi

> 
> Thanks,
> Cristian
> 
> [1]: https://lore.kernel.org/linux-arm-kernel/20240212123233.1230090-1-cristian.marussi@arm.com/T/#ma68cefd753e34ba3e1f2d4392e978026a87e1bf8
>
diff mbox series

Patch

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 211e8e0aef2c..ae7681eda276 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -962,6 +962,21 @@  scmi_power_scale_get(const struct scmi_protocol_handle *ph)
 	return pi->power_scale;
 }
 
+static int scmi_notify_support(const struct scmi_protocol_handle *ph, u32 domain,
+			       struct scmi_perf_notify_info *info)
+{
+	struct perf_dom_info *dom;
+
+	dom = scmi_perf_domain_lookup(ph, domain);
+	if (IS_ERR(dom))
+		return -EINVAL;
+
+	info->perf_limit_notify = dom->perf_limit_notify;
+	info->perf_level_notify = dom->perf_level_notify;
+
+	return 0;
+}
+
 static const struct scmi_perf_proto_ops perf_proto_ops = {
 	.num_domains_get = scmi_perf_num_domains_get,
 	.info_get = scmi_perf_info_get,
@@ -976,6 +991,7 @@  static const struct scmi_perf_proto_ops perf_proto_ops = {
 	.est_power_get = scmi_dvfs_est_power_get,
 	.fast_switch_possible = scmi_fast_switch_possible,
 	.power_scale_get = scmi_power_scale_get,
+	.perf_notify_support = scmi_notify_support,
 };
 
 static int scmi_perf_set_notify_enabled(const struct scmi_protocol_handle *ph,
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index f2f05fb42d28..b0947d004826 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -117,6 +117,11 @@  struct scmi_perf_domain_info {
 	bool set_perf;
 };
 
+struct scmi_perf_notify_info {
+	bool perf_limit_notify;
+	bool perf_level_notify;
+};
+
 /**
  * struct scmi_perf_proto_ops - represents the various operations provided
  *	by SCMI Performance Protocol
@@ -139,6 +144,7 @@  struct scmi_perf_domain_info {
  *	for a given device
  * @power_scale_mw_get: indicates if the power values provided are in milliWatts
  *	or in some other (abstract) scale
+ * @perf_notify_support: indicates if limit and level change notification is supported
  */
 struct scmi_perf_proto_ops {
 	int (*num_domains_get)(const struct scmi_protocol_handle *ph);
@@ -165,6 +171,8 @@  struct scmi_perf_proto_ops {
 	bool (*fast_switch_possible)(const struct scmi_protocol_handle *ph,
 				     u32 domain);
 	enum scmi_power_scale (*power_scale_get)(const struct scmi_protocol_handle *ph);
+	int (*perf_notify_support)(const struct scmi_protocol_handle *ph, u32 domain,
+				   struct scmi_perf_notify_info *info);
 };
 
 /**