[PATCH 2/2] drm/amdkfd: expose num_cp_queues data field to topology node

[Huang, Ray]

[AMD Official Use Only - Internal Distribution Only]

On Wed, Dec 18, 2019 at 07:40:26AM +0800, Kuehling, Felix wrote:
> See comment inline. Other than that, the series looks good to me.
> 
> On 2019-12-16 2:02, Huang Rui wrote:
> > Thunk driver would like to know the num_cp_queues data, however this data relied
> > on different asic specific. So it's better to get it from kfd driver.
> >
> > Signed-off-by: Huang Rui <ray.huang at amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
> >   drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 3 ++-
> >   2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > index cc01ccd..203c823 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > @@ -488,6 +488,8 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
> >   			dev->node_props.num_sdma_xgmi_engines);
> >   	sysfs_show_32bit_prop(buffer, "num_sdma_queues_per_engine",
> >   			dev->node_props.num_sdma_queues_per_engine);
> > +	sysfs_show_32bit_prop(buffer, "num_cp_queues",
> > +			dev->node_props.num_cp_queues);
> >   
> >   	if (dev->gpu) {
> >   		log_max_watch_addr =
> > @@ -1316,6 +1318,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
> >   	dev->node_props.num_gws = (hws_gws_support &&
> >   		dev->gpu->dqm->sched_policy != KFD_SCHED_POLICY_NO_HWS) ?
> >   		amdgpu_amdkfd_get_num_gws(dev->gpu->kgd) : 0;
> > +	dev->node_props.num_cp_queues = get_queues_num(dev->gpu->dqm);
> >   
> >   	kfd_fill_mem_clk_max_info(dev);
> >   	kfd_fill_iolink_non_crat_info(dev);
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > index 9346cc1..e447901 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > @@ -27,7 +27,7 @@
> >   #include <linux/list.h>
> >   #include "kfd_crat.h"
> >   
> > -#define KFD_TOPOLOGY_PUBLIC_NAME_SIZE 28
> > +#define KFD_TOPOLOGY_PUBLIC_NAME_SIZE 24
> 
> I don't see why you need to change the name size here. I'm not aware of 
> any requirement that the structure size cannot change. This comment 
> applies to patch 1 as well.
> 

Oh, sorry, I miss read this field as "reserved", so I kept the struct size.
Will updated in V2.

Thanks,
Ray

> Regards,
>    Felix
> 
> >   
> >   #define HSA_CAP_HOT_PLUGGABLE			0x00000001
> >   #define HSA_CAP_ATS_PRESENT			0x00000002
> > @@ -82,6 +82,7 @@ struct kfd_node_properties {
> >   	uint32_t num_sdma_engines;
> >   	uint32_t num_sdma_xgmi_engines;
> >   	uint32_t num_sdma_queues_per_engine;
> > +	uint32_t num_cp_queues;
> >   	char name[KFD_TOPOLOGY_PUBLIC_NAME_SIZE];
> >   };
> >