Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4635 bytes --]

On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
> > On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> > 
> >> In order to provide GPU inputs to TA for the Smart PC solution to work, we
> >> need to have interface between the PMF driver and the AMDGPU driver.
> >>
> >> Add the initial code path for get interface from AMDGPU.
> >>
> >> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/Makefile     |   2 +
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |   1 +
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138 ++++++++++++++++++++++++
> >>  drivers/platform/x86/amd/pmf/Kconfig    |   1 +
> >>  drivers/platform/x86/amd/pmf/core.c     |   1 +
> >>  drivers/platform/x86/amd/pmf/pmf.h      |   3 +
> >>  drivers/platform/x86/amd/pmf/spc.c      |  13 +++
> >>  drivers/platform/x86/amd/pmf/tee-if.c   |  26 +++++
> >>  include/linux/amd-pmf-io.h              |  35 ++++++
> >>  9 files changed, 220 insertions(+)
> >>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> >>  create mode 100644 include/linux/amd-pmf-io.h
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> >> index 384b798a9bad..7fafccefbd7a 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> >> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> >> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
> >>  
> >>  amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
> >>  
> >> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
> >> +
> >>  # add asic specific block
> >>  amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
> >>  	dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index a79d53bdbe13..26ffa1b4fe57 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -50,6 +50,7 @@
> >>  #include <linux/hashtable.h>
> >>  #include <linux/dma-fence.h>
> >>  #include <linux/pci.h>
> >> +#include <linux/amd-pmf-io.h>
> >>  
> >>  #include <drm/ttm/ttm_bo.h>
> >>  #include <drm/ttm/ttm_placement.h>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> >> new file mode 100644
> >> index 000000000000..ac981848df50
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> >> @@ -0,0 +1,138 @@
> >> +/*
> >> + * Copyright 2023 Advanced Micro Devices, Inc.
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person obtaining a
> >> + * copy of this software and associated documentation files (the "Software"),
> >> + * to deal in the Software without restriction, including without limitation
> >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> >> + * and/or sell copies of the Software, and to permit persons to whom the
> >> + * Software is furnished to do so, subject to the following conditions:
> >> + *
> >> + * The above copyright notice and this permission notice shall be included in
> >> + * all copies or substantial portions of the Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >> + * OTHER DEALINGS IN THE SOFTWARE.
> > 
> > This is MIT, right? Add the required SPDX-License-Identifier line for it
> > at the top of the file, thank you.
> > 
> 
> all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
> text. So, have retained it to maintain uniformity.

What does adding SPDX identifier line at the top of the file take away 
from that? I'd be fine if you want to add the identifier line to all of 
them too to keep them identical.

> >> + *
> >> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> + *
> >> + */
> > 
> > Remove the extra empty line at the end of the comment.
> > 
> 
> I just took the standard template for all the gpu files. Is that OK to
> retain the blank line?
> 
> If not, I can remove it in the next version.

I don't want to fight over a blank line when you insist on keeping it :-).

-- 
 i.