Hi Christian,

I mean patch 5 can be changed as

static struct amd_vce_state*  pp_dpm_get_vce_clock_table(void *handle,  int num)
{
    PP_CHECK((struct pp_instance *)handle);
    hwmgr = ((struct pp_instance *)handle)->hwmgr;

if (hwmgr && num < hwmgr->num_vce_state_tables)

     return &hwmgr->vce_states[i];

    Return NULL;
}

And in Patch6.


+       case AMDGPU_INFO_VCE_CLOCK_TABLE: {

+                struct drm_amdgpu_info_vce_clock_table vce_clk_table = {};

                 struct amd_vce_state*  vce_state;



+      for (i = 0; i < AMDGPU_VCE_CLOCK_TABLE_ENTRIES; i++) {

                 vce_state = amdgpu_dpm_get_vce_clock_table(adev, i);

                         If (vce_state) {

                                      vce_clk_table.entries[i].sclk = 
vce_state->sclk;
             vce_clk_table.entries[i].mclk = vce_state->mclk;
            vce_clk_table.entries[i].eclk = vce_state->evclk;

                        } else {

                               vce_clk_table.num_of_entries = i;         //if 
UMD driver need to check number of vce state.

                               break;

                      }

              }

+                return copy_to_user(out, &vce_clk_table,

+                                       min((size_t)size, 
sizeof(vce_clk_table))) ? -EFAULT : 0;

+       }



Best Regards
Rex
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Christian König
Sent: Wednesday, October 12, 2016 4:53 PM
To: Zhu, Rex; Alex Deucher; amd-gfx list; Fang, Peter; Zhang, Boyuan
Cc: Deucher, Alexander
Subject: Re: [PATCH 5/6] drm/amdgpu/powerplay: add an implementation for 
get_vce_clock_table

Hi Rex,

the problem with this approach is that you need to copy the entries from one 
structure to another, like we have below:

> +                               vce_clk_table.entries[i].sclk = 
> hwmgr->vce_states[i].sclk;
> +                               vce_clk_table.entries[i].mclk = 
> hwmgr->vce_states[i].mclk;
> +                               vce_clk_table.entries[i].eclk = 
> hwmgr->vce_states[i].evclk;

This is not seen as good coding practice and usually isn't allowed upstream 
because it creates two representations in the kernel for the same thing.

Regards,
Christian.

Am 12.10.2016 um 10:34 schrieb Zhu, Rex:

Hi Alex,



In Patch1, I moved struct amdgpu_vce_states to amd_shared.h from amdgpu.h. So 
in powerplay, we used same vce state definition.



So the define of static struct drm_amdgpu_info_vce_clock_table  
pp_dpm_get_vce_clock_table(void *handle)

Can change to

static struct amd_vce_state  pp_dpm_get_vce_clock_table(void *handle,  int num);



and can move those codes to Patch6.

+                               vce_clk_table.entries[i].sclk = 
hwmgr->vce_states[i].sclk;

+                               vce_clk_table.entries[i].mclk = 
hwmgr->vce_states[i].mclk;

+                               vce_clk_table.entries[i].eclk = 
hwmgr->vce_states[i].evclk;





So in powerplay and dpm, we don't need to visit struct 
drm_amdgpu_info_vce_clock_table.



In Patch2, add numer of vce_states in struct dpm. If it is useful to UMD 
driver. We can notify UMD by add a member in struct 
drm_amdgpu_info_vce_clock_table.



Best Regards

Rex



-----Original Message-----

From: Alex Deucher [mailto:alexdeuc...@gmail.com]

Sent: Wednesday, October 12, 2016 4:07 AM

To: amd-gfx list; Fang, Peter; Zhang, Boyuan; Zhu, Rex

Cc: Deucher, Alexander

Subject: Re: [PATCH 5/6] drm/amdgpu/powerplay: add an implementation for 
get_vce_clock_table



Rex what is your opinion on exposing the amdgpu drm structure through to 
powerplay?  We could do an intermediate interface, but that just seems like 
extra hoops to jump through for pretty questionable gain.



Alex



On Fri, Oct 7, 2016 at 2:28 PM, Alex Deucher 
<alexdeuc...@gmail.com><mailto:alexdeuc...@gmail.com> wrote:

Used by the powerplay dpm code.



Signed-off-by: Alex Deucher 
<alexander.deuc...@amd.com><mailto:alexander.deuc...@amd.com>

---

 drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 24

++++++++++++++++++++++++

 1 file changed, 24 insertions(+)



diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

index bb8a345..8eee390 100644

--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c

@@ -30,6 +30,7 @@

 #include "power_state.h"

 #include "eventmanager.h"

 #include "pp_debug.h"

+#include "drm/amdgpu_drm.h"





 #define PP_CHECK(handle)                                               \

@@ -821,6 +822,28 @@ static int pp_dpm_read_sensor(void *handle, int idx, 
int32_t *value)

        return hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value);  }



+static struct drm_amdgpu_info_vce_clock_table

+pp_dpm_get_vce_clock_table(void *handle) {

+       struct drm_amdgpu_info_vce_clock_table vce_clk_table = {};

+       struct pp_hwmgr *hwmgr;

+       unsigned i;

+

+       if (handle) {

+               hwmgr = ((struct pp_instance *)handle)->hwmgr;

+

+               if (hwmgr) {

+                       for (i = 0; i < AMDGPU_VCE_CLOCK_TABLE_ENTRIES; i++) {

+                               vce_clk_table.entries[i].sclk = 
hwmgr->vce_states[i].sclk;

+                               vce_clk_table.entries[i].mclk = 
hwmgr->vce_states[i].mclk;

+                               vce_clk_table.entries[i].eclk = 
hwmgr->vce_states[i].evclk;

+                       }

+               }

+       }

+

+       return vce_clk_table;

+}

+

 const struct amd_powerplay_funcs pp_dpm_funcs = {

        .get_temperature = pp_dpm_get_temperature,

        .load_firmware = pp_dpm_load_fw, @@ -847,6 +870,7 @@ const

struct amd_powerplay_funcs pp_dpm_funcs = {

        .get_mclk_od = pp_dpm_get_mclk_od,

        .set_mclk_od = pp_dpm_set_mclk_od,

        .read_sensor = pp_dpm_read_sensor,

+       .get_vce_clock_table = pp_dpm_get_vce_clock_table,

 };



 static int amd_pp_instance_init(struct amd_pp_init *pp_init,

--

2.5.5






_______________________________________________

amd-gfx mailing list

amd-gfx@lists.freedesktop.org<mailto: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

Reply via email to