Re: [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation
On 2020-06-01 3:30 a.m., Evan Quan wrote:
> Since the structure comes with only several bytes.
>
This is not a good commit message as it doesn't describe
what is being done. It evokes the "Yes? Then what?" questions
from a reader.
Perhaps a better one would be:
Allocate the struct amdgpu_irq_src on the stack,
since it is only several bytes in size.
Regards,
Luben
> Change-Id: Ie9df0db543fdd4cf5b963a286ef40dee03c436bf
> Signed-off-by: Evan Quan
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 3 ---
> drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +++
> 3 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index e55c6458b212..b353ac1b0f07 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1121,9 +1121,6 @@ static int smu_sw_fini(void *handle)
> struct smu_context *smu = &adev->smu;
> int ret;
>
> - kfree(smu->irq_source);
> - smu->irq_source = NULL;
> -
> ret = smu_smc_table_sw_fini(smu);
> if (ret) {
> pr_err("Failed to sw fini smc table!\n");
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 4aa63dc79124..7fed2556213f 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -356,7 +356,7 @@ struct smu_baco_context
> struct smu_context
> {
> struct amdgpu_device*adev;
> - struct amdgpu_irq_src *irq_source;
> + struct amdgpu_irq_src irq_source;
>
> const struct pptable_funcs *ppt_funcs;
> struct mutexmutex;
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 891781a5c0d4..e2b1c619151f 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1167,7 +1167,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context
> *smu)
> if (ret)
> return ret;
>
> - ret = amdgpu_irq_get(adev, smu->irq_source, 0);
> + ret = amdgpu_irq_get(adev, &smu->irq_source, 0);
> if (ret)
> return ret;
>
> @@ -1191,7 +1191,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context
> *smu)
>
> int smu_v11_0_disable_thermal_alert(struct smu_context *smu)
> {
> - return amdgpu_irq_put(smu->adev, smu->irq_source, 0);
> + return amdgpu_irq_put(smu->adev, &smu->irq_source, 0);
> }
>
> static uint16_t convert_to_vddc(uint8_t vid)
> @@ -1607,18 +1607,9 @@ static const struct amdgpu_irq_src_funcs
> smu_v11_0_irq_funcs =
> int smu_v11_0_register_irq_handler(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> - struct amdgpu_irq_src *irq_src = smu->irq_source;
> + struct amdgpu_irq_src *irq_src = &smu->irq_source;
> int ret = 0;
>
> - /* already register */
> - if (irq_src)
> - return 0;
> -
> - irq_src = kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
> - if (!irq_src)
> - return -ENOMEM;
> - smu->irq_source = irq_src;
> -
> irq_src->num_types = 1;
> irq_src->funcs = &smu_v11_0_irq_funcs;
>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation
On Mon, Jun 1, 2020 at 3:31 AM Evan Quan wrote:
>
> Since the structure comes with only several bytes.
>
> Change-Id: Ie9df0db543fdd4cf5b963a286ef40dee03c436bf
> Signed-off-by: Evan Quan
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 3 ---
> drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +++
> 3 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index e55c6458b212..b353ac1b0f07 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -1121,9 +1121,6 @@ static int smu_sw_fini(void *handle)
> struct smu_context *smu = &adev->smu;
> int ret;
>
> - kfree(smu->irq_source);
> - smu->irq_source = NULL;
> -
> ret = smu_smc_table_sw_fini(smu);
> if (ret) {
> pr_err("Failed to sw fini smc table!\n");
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 4aa63dc79124..7fed2556213f 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -356,7 +356,7 @@ struct smu_baco_context
> struct smu_context
> {
> struct amdgpu_device*adev;
> - struct amdgpu_irq_src *irq_source;
> + struct amdgpu_irq_src irq_source;
>
> const struct pptable_funcs *ppt_funcs;
> struct mutexmutex;
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 891781a5c0d4..e2b1c619151f 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1167,7 +1167,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context
> *smu)
> if (ret)
> return ret;
>
> - ret = amdgpu_irq_get(adev, smu->irq_source, 0);
> + ret = amdgpu_irq_get(adev, &smu->irq_source, 0);
> if (ret)
> return ret;
>
> @@ -1191,7 +1191,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context
> *smu)
>
> int smu_v11_0_disable_thermal_alert(struct smu_context *smu)
> {
> - return amdgpu_irq_put(smu->adev, smu->irq_source, 0);
> + return amdgpu_irq_put(smu->adev, &smu->irq_source, 0);
> }
>
> static uint16_t convert_to_vddc(uint8_t vid)
> @@ -1607,18 +1607,9 @@ static const struct amdgpu_irq_src_funcs
> smu_v11_0_irq_funcs =
> int smu_v11_0_register_irq_handler(struct smu_context *smu)
> {
> struct amdgpu_device *adev = smu->adev;
> - struct amdgpu_irq_src *irq_src = smu->irq_source;
> + struct amdgpu_irq_src *irq_src = &smu->irq_source;
> int ret = 0;
>
> - /* already register */
> - if (irq_src)
> - return 0;
> -
> - irq_src = kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
> - if (!irq_src)
> - return -ENOMEM;
> - smu->irq_source = irq_src;
> -
> irq_src->num_types = 1;
> irq_src->funcs = &smu_v11_0_irq_funcs;
>
> --
> 2.26.2
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 8/9] drm/amd/powerplay: drop unnecessary dynamic buffer allocation
Since the structure comes with only several bytes.
Change-Id: Ie9df0db543fdd4cf5b963a286ef40dee03c436bf
Signed-off-by: Evan Quan
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 3 ---
drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 +-
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 15 +++
3 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index e55c6458b212..b353ac1b0f07 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1121,9 +1121,6 @@ static int smu_sw_fini(void *handle)
struct smu_context *smu = &adev->smu;
int ret;
- kfree(smu->irq_source);
- smu->irq_source = NULL;
-
ret = smu_smc_table_sw_fini(smu);
if (ret) {
pr_err("Failed to sw fini smc table!\n");
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 4aa63dc79124..7fed2556213f 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -356,7 +356,7 @@ struct smu_baco_context
struct smu_context
{
struct amdgpu_device*adev;
- struct amdgpu_irq_src *irq_source;
+ struct amdgpu_irq_src irq_source;
const struct pptable_funcs *ppt_funcs;
struct mutexmutex;
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 891781a5c0d4..e2b1c619151f 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1167,7 +1167,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context
*smu)
if (ret)
return ret;
- ret = amdgpu_irq_get(adev, smu->irq_source, 0);
+ ret = amdgpu_irq_get(adev, &smu->irq_source, 0);
if (ret)
return ret;
@@ -1191,7 +1191,7 @@ int smu_v11_0_enable_thermal_alert(struct smu_context
*smu)
int smu_v11_0_disable_thermal_alert(struct smu_context *smu)
{
- return amdgpu_irq_put(smu->adev, smu->irq_source, 0);
+ return amdgpu_irq_put(smu->adev, &smu->irq_source, 0);
}
static uint16_t convert_to_vddc(uint8_t vid)
@@ -1607,18 +1607,9 @@ static const struct amdgpu_irq_src_funcs
smu_v11_0_irq_funcs =
int smu_v11_0_register_irq_handler(struct smu_context *smu)
{
struct amdgpu_device *adev = smu->adev;
- struct amdgpu_irq_src *irq_src = smu->irq_source;
+ struct amdgpu_irq_src *irq_src = &smu->irq_source;
int ret = 0;
- /* already register */
- if (irq_src)
- return 0;
-
- irq_src = kzalloc(sizeof(struct amdgpu_irq_src), GFP_KERNEL);
- if (!irq_src)
- return -ENOMEM;
- smu->irq_source = irq_src;
-
irq_src->num_types = 1;
irq_src->funcs = &smu_v11_0_irq_funcs;
--
2.26.2
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
