diff mbox series

[04/16] drm/i915/guc/slpc: Lay out slpc init/enable/disable/fini

Message ID 20210710012026.19705-5-vinay.belgaumkar@intel.com (mailing list archive)
State New, archived
Headers show
Series Enable GuC based power management features | expand

Commit Message

Vinay Belgaumkar July 10, 2021, 1:20 a.m. UTC 
Declare header and source files for SLPC, along with init and
enable/disable function templates.

Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Signed-off-by: Sundaresan Sujaritha <sujaritha.sundaresan@intel.com>
---
 drivers/gpu/drm/i915/Makefile               |  1 +
 drivers/gpu/drm/i915/gt/uc/intel_guc.h      |  2 ++
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 34 +++++++++++++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 16 ++++++++++
 4 files changed, 53 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
 create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h

Comments

Michal Wajdeczko July 10, 2021, 2:35 p.m. UTC | #1 
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Declare header and source files for SLPC, along with init and
> enable/disable function templates.

later you claim that "disable" is not needed

> 
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Signed-off-by: Sundaresan Sujaritha <sujaritha.sundaresan@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile               |  1 +
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h      |  2 ++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 34 +++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 16 ++++++++++
>  4 files changed, 53 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index ab7679957623..d8eac4468df9 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -186,6 +186,7 @@ i915-y += gt/uc/intel_uc.o \
>  	  gt/uc/intel_guc_fw.o \
>  	  gt/uc/intel_guc_log.o \
>  	  gt/uc/intel_guc_log_debugfs.o \
> +	  gt/uc/intel_guc_slpc.o \
>  	  gt/uc/intel_guc_submission.o \
>  	  gt/uc/intel_huc.o \
>  	  gt/uc/intel_huc_debugfs.o \
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index e5a456918b88..0dbbd9cf553f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -15,6 +15,7 @@
>  #include "intel_guc_ct.h"
>  #include "intel_guc_log.h"
>  #include "intel_guc_reg.h"
> +#include "intel_guc_slpc.h"
>  #include "intel_uc_fw.h"
>  #include "i915_utils.h"
>  #include "i915_vma.h"
> @@ -30,6 +31,7 @@ struct intel_guc {
>  	struct intel_uc_fw fw;
>  	struct intel_guc_log log;
>  	struct intel_guc_ct ct;
> +	struct intel_guc_slpc slpc;
>  
>  	/* Global engine used to submit requests to GuC */
>  	struct i915_sched_engine *sched_engine;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> new file mode 100644
> index 000000000000..c1f569d2300d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -0,0 +1,34 @@
> +/*
> + * SPDX-License-Identifier: MIT

SPDX tag shall be in very first line, for .c:

// SPDX-License-Identifier: MIT

> + *
> + * Copyright © 2020 Intel Corporation

2021

> + */
> +
> +#include "intel_guc_slpc.h"
> +
> +int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
> +{
> +	return 0;
> +}
> +
> +/*
> + * intel_guc_slpc_enable() - Start SLPC
> + * @slpc: pointer to intel_guc_slpc.
> + *
> + * SLPC is enabled by setting up the shared data structure and
> + * sending reset event to GuC SLPC. Initial data is setup in
> + * intel_guc_slpc_init. Here we send the reset event. We do
> + * not currently need a slpc_disable since this is taken care
> + * of automatically when a reset/suspend occurs and the guc

s/guc/GuC

> + * channels are destroyed.

you mean CTB ?

> + *
> + * Return: 0 on success, non-zero error code on failure.
> + */
> +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
> +{
> +	return 0;
> +}
> +
> +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc)
> +{
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> new file mode 100644
> index 000000000000..74fd86769163
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -0,0 +1,16 @@
> +/*
> + * SPDX-License-Identifier: MIT

SPDX tag shall be in very first line, for .h:

/* SPDX-License-Identifier: MIT */

> + *
> + * Copyright © 2020 Intel Corporation

2021

> + */
> +#ifndef _INTEL_GUC_SLPC_H_
> +#define _INTEL_GUC_SLPC_H_
> +
> +struct intel_guc_slpc {
> +};

move all data definitions to intel_guc_slpc_types.h and include it here

> +
> +int intel_guc_slpc_init(struct intel_guc_slpc *slpc);
> +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc);
> +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc);
> +
> +#endif
> 

and as suggested in comment to 2/14 you should likely move this patch to
the front of the series

Michal
Vinay Belgaumkar July 13, 2021, 12:37 a.m. UTC | #2 
On 7/10/2021 7:35 AM, Michal Wajdeczko wrote:
> 
> 
> On 10.07.2021 03:20, Vinay Belgaumkar wrote:
>> Declare header and source files for SLPC, along with init and
>> enable/disable function templates.
> 
> later you claim that "disable" is not needed

Changed.

> 
>>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Signed-off-by: Sundaresan Sujaritha <sujaritha.sundaresan@intel.com>
>> ---
>>   drivers/gpu/drm/i915/Makefile               |  1 +
>>   drivers/gpu/drm/i915/gt/uc/intel_guc.h      |  2 ++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 34 +++++++++++++++++++++
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 16 ++++++++++
>>   4 files changed, 53 insertions(+)
>>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>   create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index ab7679957623..d8eac4468df9 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -186,6 +186,7 @@ i915-y += gt/uc/intel_uc.o \
>>   	  gt/uc/intel_guc_fw.o \
>>   	  gt/uc/intel_guc_log.o \
>>   	  gt/uc/intel_guc_log_debugfs.o \
>> +	  gt/uc/intel_guc_slpc.o \
>>   	  gt/uc/intel_guc_submission.o \
>>   	  gt/uc/intel_huc.o \
>>   	  gt/uc/intel_huc_debugfs.o \
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> index e5a456918b88..0dbbd9cf553f 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> @@ -15,6 +15,7 @@
>>   #include "intel_guc_ct.h"
>>   #include "intel_guc_log.h"
>>   #include "intel_guc_reg.h"
>> +#include "intel_guc_slpc.h"
>>   #include "intel_uc_fw.h"
>>   #include "i915_utils.h"
>>   #include "i915_vma.h"
>> @@ -30,6 +31,7 @@ struct intel_guc {
>>   	struct intel_uc_fw fw;
>>   	struct intel_guc_log log;
>>   	struct intel_guc_ct ct;
>> +	struct intel_guc_slpc slpc;
>>   
>>   	/* Global engine used to submit requests to GuC */
>>   	struct i915_sched_engine *sched_engine;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> new file mode 100644
>> index 000000000000..c1f569d2300d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> @@ -0,0 +1,34 @@
>> +/*
>> + * SPDX-License-Identifier: MIT
> 
> SPDX tag shall be in very first line, for .c:
> 
> // SPDX-License-Identifier: MIT
> 
>> + *
>> + * Copyright © 2020 Intel Corporation
> 
> 2021

done.

> 
>> + */
>> +
>> +#include "intel_guc_slpc.h"
>> +
>> +int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
>> +{
>> +	return 0;
>> +}
>> +
>> +/*
>> + * intel_guc_slpc_enable() - Start SLPC
>> + * @slpc: pointer to intel_guc_slpc.
>> + *
>> + * SLPC is enabled by setting up the shared data structure and
>> + * sending reset event to GuC SLPC. Initial data is setup in
>> + * intel_guc_slpc_init. Here we send the reset event. We do
>> + * not currently need a slpc_disable since this is taken care
>> + * of automatically when a reset/suspend occurs and the guc
> 
> s/guc/GuC
> 
>> + * channels are destroyed.
> 
> you mean CTB ?

yes, fixed.

> 
>> + *
>> + * Return: 0 on success, non-zero error code on failure.
>> + */
>> +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>> +{
>> +	return 0;
>> +}
>> +
>> +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc)
>> +{
>> +}
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
>> new file mode 100644
>> index 000000000000..74fd86769163
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * SPDX-License-Identifier: MIT
> 
> SPDX tag shall be in very first line, for .h:
> 
> /* SPDX-License-Identifier: MIT */
> 
>> + *
>> + * Copyright © 2020 Intel Corporation
> 
> 2021
> 
>> + */
>> +#ifndef _INTEL_GUC_SLPC_H_
>> +#define _INTEL_GUC_SLPC_H_
>> +
>> +struct intel_guc_slpc {
>> +};
> 
> move all data definitions to intel_guc_slpc_types.h and include it here
> 
>> +
>> +int intel_guc_slpc_init(struct intel_guc_slpc *slpc);
>> +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc);
>> +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc);
>> +
>> +#endif
>>
> 
> and as suggested in comment to 2/14 you should likely move this patch to
> the front of the series

Yes, squashed with the first patch.

Thanks,
Vinay.

> 
> Michal
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index ab7679957623..d8eac4468df9 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -186,6 +186,7 @@  i915-y += gt/uc/intel_uc.o \
 	  gt/uc/intel_guc_fw.o \
 	  gt/uc/intel_guc_log.o \
 	  gt/uc/intel_guc_log_debugfs.o \
+	  gt/uc/intel_guc_slpc.o \
 	  gt/uc/intel_guc_submission.o \
 	  gt/uc/intel_huc.o \
 	  gt/uc/intel_huc_debugfs.o \
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index e5a456918b88..0dbbd9cf553f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -15,6 +15,7 @@ 
 #include "intel_guc_ct.h"
 #include "intel_guc_log.h"
 #include "intel_guc_reg.h"
+#include "intel_guc_slpc.h"
 #include "intel_uc_fw.h"
 #include "i915_utils.h"
 #include "i915_vma.h"
@@ -30,6 +31,7 @@  struct intel_guc {
 	struct intel_uc_fw fw;
 	struct intel_guc_log log;
 	struct intel_guc_ct ct;
+	struct intel_guc_slpc slpc;
 
 	/* Global engine used to submit requests to GuC */
 	struct i915_sched_engine *sched_engine;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
new file mode 100644
index 000000000000..c1f569d2300d
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -0,0 +1,34 @@ 
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2020 Intel Corporation
+ */
+
+#include "intel_guc_slpc.h"
+
+int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
+{
+	return 0;
+}
+
+/*
+ * intel_guc_slpc_enable() - Start SLPC
+ * @slpc: pointer to intel_guc_slpc.
+ *
+ * SLPC is enabled by setting up the shared data structure and
+ * sending reset event to GuC SLPC. Initial data is setup in
+ * intel_guc_slpc_init. Here we send the reset event. We do
+ * not currently need a slpc_disable since this is taken care
+ * of automatically when a reset/suspend occurs and the guc
+ * channels are destroyed.
+ *
+ * Return: 0 on success, non-zero error code on failure.
+ */
+int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
+{
+	return 0;
+}
+
+void intel_guc_slpc_fini(struct intel_guc_slpc *slpc)
+{
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
new file mode 100644
index 000000000000..74fd86769163
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
@@ -0,0 +1,16 @@ 
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2020 Intel Corporation
+ */
+#ifndef _INTEL_GUC_SLPC_H_
+#define _INTEL_GUC_SLPC_H_
+
+struct intel_guc_slpc {
+};
+
+int intel_guc_slpc_init(struct intel_guc_slpc *slpc);
+int intel_guc_slpc_enable(struct intel_guc_slpc *slpc);
+void intel_guc_slpc_fini(struct intel_guc_slpc *slpc);
+
+#endif