RE: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage

2016-12-01 Thread Yu, Xiangliang 
> On Thu, Dec 1, 2016 at 1:53 AM, Yu, Xiangliang 
> wrote:
> >>
> >> > -Original Message-
> >> > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On
> >> > Behalf Of Xiangliang Yu
> >> > Sent: Wednesday, November 30, 2016 2:02 AM
> >> > To: amd-gfx@lists.freedesktop.org
> >> > Cc: Yu, Xiangliang; Liu, Monk
> >> > Subject: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage
> >> >
> >> > SMU buffer is used for power feature, but for virtualization, the
> >> > power is controlled by hypervisor. Ignore it.
> >> >
> >> > Signed-off-by: Xiangliang Yu 
> >> > Signed-off-by: Monk Liu 
> >> > ---
> >> >  drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 15
> >> > ---
> >> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> >> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> >> > index 877445d..f49b548 100644
> >> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> >> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> >> > @@ -407,8 +407,14 @@ int smu7_request_smu_load_fw(struct
> >> pp_smumgr
> >> > *smumgr)
> >> > 0x0);
> >> >
> >> > if (smumgr->chip_id > CHIP_TOPAZ) { /* add support for Topaz */
> >> > -   smu7_send_msg_to_smc_with_parameter(smumgr,
> >> > PPSMC_MSG_SMU_DRAM_ADDR_HI, smu_data-
> >> > >smu_buffer.mc_addr_high);
> >> > -   smu7_send_msg_to_smc_with_parameter(smumgr,
> >> > PPSMC_MSG_SMU_DRAM_ADDR_LO, smu_data-
> >> > >smu_buffer.mc_addr_low);
> >> > +   if (!cgs_is_virtualization_enabled(smumgr->device)) {
> >> > +   smu7_send_msg_to_smc_with_parameter(smumgr,
> >> > +
> >> > PPSMC_MSG_SMU_DRAM_ADDR_HI,
> >> > +   smu_data-
> >> > >smu_buffer.mc_addr_high);
> >> > +   smu7_send_msg_to_smc_with_parameter(smumgr,
> >> > +
> >> > PPSMC_MSG_SMU_DRAM_ADDR_LO,
> >> > +   smu_data-
> >> > >smu_buffer.mc_addr_low);
> >> > +   }
> >> > fw_to_load = UCODE_ID_RLC_G_MASK
> >> >+ UCODE_ID_SDMA0_MASK
> >> >+ UCODE_ID_SDMA1_MASK @@ -543,7 +549,6 @@
> >> > int smu7_init(struct pp_smumgr *smumgr)
> >> > smu_data = (struct smu7_smumgr *)(smumgr->backend);
> >> > smu_data->header_buffer.data_size =
> >> > ((sizeof(struct SMU_DRAMData_TOC) / 4096) + 1)
> >> > *
> >> 4096;
> >> > -   smu_data->smu_buffer.data_size = 200*4096;
> >>
> >> Was this change intended?  I'm assuming this is extraneous as it's
> >> set in
> >> smu7_init() as well.
> >
> > It still in smu7_init() function, just put it together. I think it make 
> > code more
> clear.
> >
> 
> That should probably be a separate commit since it's a different logical
> change.  With that fixed, this patch is:
> Reviewed-by: Alex Deucher 

Ok, I'll submit V2 later.

> >>
> >> >
> >> >  /* Allocate FW image data structure and header buffer and
> >> >   * send the header buffer address to SMU */ @@ -566,6 +571,10 @@
> >> > int smu7_init(struct pp_smumgr *smumgr)
> >> > (cgs_handle_t)smu_data->header_buffer.handle);
> >> > return -EINVAL);
> >> >
> >> > +   if (cgs_is_virtualization_enabled(smumgr->device))
> >> > +   return 0;
> >> > +
> >> > +   smu_data->smu_buffer.data_size = 200*4096;
> >> > smu_allocate_memory(smumgr->device,
> >> > smu_data->smu_buffer.data_size,
> >> > CGS_GPU_MEM_TYPE__VISIBLE_CONTIG_FB,
> >> > --
> >> > 2.7.4
> >> >
> >> > ___
> >> > 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
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage

2016-12-01 Thread Alex Deucher 
On Thu, Dec 1, 2016 at 1:53 AM, Yu, Xiangliang  wrote:
>>
>> > -Original Message-
>> > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
>> > Of Xiangliang Yu
>> > Sent: Wednesday, November 30, 2016 2:02 AM
>> > To: amd-gfx@lists.freedesktop.org
>> > Cc: Yu, Xiangliang; Liu, Monk
>> > Subject: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage
>> >
>> > SMU buffer is used for power feature, but for virtualization, the
>> > power is controlled by hypervisor. Ignore it.
>> >
>> > Signed-off-by: Xiangliang Yu 
>> > Signed-off-by: Monk Liu 
>> > ---
>> >  drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 15
>> > ---
>> >  1 file changed, 12 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
>> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
>> > index 877445d..f49b548 100644
>> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
>> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
>> > @@ -407,8 +407,14 @@ int smu7_request_smu_load_fw(struct
>> pp_smumgr
>> > *smumgr)
>> > 0x0);
>> >
>> > if (smumgr->chip_id > CHIP_TOPAZ) { /* add support for Topaz */
>> > -   smu7_send_msg_to_smc_with_parameter(smumgr,
>> > PPSMC_MSG_SMU_DRAM_ADDR_HI, smu_data-
>> > >smu_buffer.mc_addr_high);
>> > -   smu7_send_msg_to_smc_with_parameter(smumgr,
>> > PPSMC_MSG_SMU_DRAM_ADDR_LO, smu_data-
>> > >smu_buffer.mc_addr_low);
>> > +   if (!cgs_is_virtualization_enabled(smumgr->device)) {
>> > +   smu7_send_msg_to_smc_with_parameter(smumgr,
>> > +
>> > PPSMC_MSG_SMU_DRAM_ADDR_HI,
>> > +   smu_data-
>> > >smu_buffer.mc_addr_high);
>> > +   smu7_send_msg_to_smc_with_parameter(smumgr,
>> > +
>> > PPSMC_MSG_SMU_DRAM_ADDR_LO,
>> > +   smu_data-
>> > >smu_buffer.mc_addr_low);
>> > +   }
>> > fw_to_load = UCODE_ID_RLC_G_MASK
>> >+ UCODE_ID_SDMA0_MASK
>> >+ UCODE_ID_SDMA1_MASK
>> > @@ -543,7 +549,6 @@ int smu7_init(struct pp_smumgr *smumgr)
>> > smu_data = (struct smu7_smumgr *)(smumgr->backend);
>> > smu_data->header_buffer.data_size =
>> > ((sizeof(struct SMU_DRAMData_TOC) / 4096) + 1) *
>> 4096;
>> > -   smu_data->smu_buffer.data_size = 200*4096;
>>
>> Was this change intended?  I'm assuming this is extraneous as it's set in
>> smu7_init() as well.
>
> It still in smu7_init() function, just put it together. I think it make code 
> more clear.
>

That should probably be a separate commit since it's a different
logical change.  With that fixed, this patch is:
Reviewed-by: Alex Deucher 

>>
>> >
>> >  /* Allocate FW image data structure and header buffer and
>> >   * send the header buffer address to SMU */ @@ -566,6 +571,10 @@ int
>> > smu7_init(struct pp_smumgr *smumgr)
>> > (cgs_handle_t)smu_data->header_buffer.handle);
>> > return -EINVAL);
>> >
>> > +   if (cgs_is_virtualization_enabled(smumgr->device))
>> > +   return 0;
>> > +
>> > +   smu_data->smu_buffer.data_size = 200*4096;
>> > smu_allocate_memory(smumgr->device,
>> > smu_data->smu_buffer.data_size,
>> > CGS_GPU_MEM_TYPE__VISIBLE_CONTIG_FB,
>> > --
>> > 2.7.4
>> >
>> > ___
>> > 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
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage

2016-11-30 Thread Yu, Xiangliang 
> 
> > -Original Message-
> > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> > Of Xiangliang Yu
> > Sent: Wednesday, November 30, 2016 2:02 AM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Yu, Xiangliang; Liu, Monk
> > Subject: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage
> >
> > SMU buffer is used for power feature, but for virtualization, the
> > power is controlled by hypervisor. Ignore it.
> >
> > Signed-off-by: Xiangliang Yu 
> > Signed-off-by: Monk Liu 
> > ---
> >  drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 15
> > ---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > index 877445d..f49b548 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > @@ -407,8 +407,14 @@ int smu7_request_smu_load_fw(struct
> pp_smumgr
> > *smumgr)
> > 0x0);
> >
> > if (smumgr->chip_id > CHIP_TOPAZ) { /* add support for Topaz */
> > -   smu7_send_msg_to_smc_with_parameter(smumgr,
> > PPSMC_MSG_SMU_DRAM_ADDR_HI, smu_data-
> > >smu_buffer.mc_addr_high);
> > -   smu7_send_msg_to_smc_with_parameter(smumgr,
> > PPSMC_MSG_SMU_DRAM_ADDR_LO, smu_data-
> > >smu_buffer.mc_addr_low);
> > +   if (!cgs_is_virtualization_enabled(smumgr->device)) {
> > +   smu7_send_msg_to_smc_with_parameter(smumgr,
> > +
> > PPSMC_MSG_SMU_DRAM_ADDR_HI,
> > +   smu_data-
> > >smu_buffer.mc_addr_high);
> > +   smu7_send_msg_to_smc_with_parameter(smumgr,
> > +
> > PPSMC_MSG_SMU_DRAM_ADDR_LO,
> > +   smu_data-
> > >smu_buffer.mc_addr_low);
> > +   }
> > fw_to_load = UCODE_ID_RLC_G_MASK
> >+ UCODE_ID_SDMA0_MASK
> >+ UCODE_ID_SDMA1_MASK
> > @@ -543,7 +549,6 @@ int smu7_init(struct pp_smumgr *smumgr)
> > smu_data = (struct smu7_smumgr *)(smumgr->backend);
> > smu_data->header_buffer.data_size =
> > ((sizeof(struct SMU_DRAMData_TOC) / 4096) + 1) *
> 4096;
> > -   smu_data->smu_buffer.data_size = 200*4096;
> 
> Was this change intended?  I'm assuming this is extraneous as it's set in
> smu7_init() as well.

It still in smu7_init() function, just put it together. I think it make code 
more clear.

> 
> >
> >  /* Allocate FW image data structure and header buffer and
> >   * send the header buffer address to SMU */ @@ -566,6 +571,10 @@ int
> > smu7_init(struct pp_smumgr *smumgr)
> > (cgs_handle_t)smu_data->header_buffer.handle);
> > return -EINVAL);
> >
> > +   if (cgs_is_virtualization_enabled(smumgr->device))
> > +   return 0;
> > +
> > +   smu_data->smu_buffer.data_size = 200*4096;
> > smu_allocate_memory(smumgr->device,
> > smu_data->smu_buffer.data_size,
> > CGS_GPU_MEM_TYPE__VISIBLE_CONTIG_FB,
> > --
> > 2.7.4
> >
> > ___
> > 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

RE: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage

2016-11-30 Thread Deucher, Alexander 


> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Xiangliang Yu
> Sent: Wednesday, November 30, 2016 2:02 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Yu, Xiangliang; Liu, Monk
> Subject: [PATCH 3/6] drm/amd/powerplay: Ignore smu buffer usage
> 
> SMU buffer is used for power feature, but for virtualization, the
> power is controlled by hypervisor. Ignore it.
> 
> Signed-off-by: Xiangliang Yu 
> Signed-off-by: Monk Liu 
> ---
>  drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 15
> ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> index 877445d..f49b548 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> @@ -407,8 +407,14 @@ int smu7_request_smu_load_fw(struct pp_smumgr
> *smumgr)
>   0x0);
> 
>   if (smumgr->chip_id > CHIP_TOPAZ) { /* add support for Topaz */
> - smu7_send_msg_to_smc_with_parameter(smumgr,
> PPSMC_MSG_SMU_DRAM_ADDR_HI, smu_data-
> >smu_buffer.mc_addr_high);
> - smu7_send_msg_to_smc_with_parameter(smumgr,
> PPSMC_MSG_SMU_DRAM_ADDR_LO, smu_data-
> >smu_buffer.mc_addr_low);
> + if (!cgs_is_virtualization_enabled(smumgr->device)) {
> + smu7_send_msg_to_smc_with_parameter(smumgr,
> +
>   PPSMC_MSG_SMU_DRAM_ADDR_HI,
> + smu_data-
> >smu_buffer.mc_addr_high);
> + smu7_send_msg_to_smc_with_parameter(smumgr,
> +
>   PPSMC_MSG_SMU_DRAM_ADDR_LO,
> + smu_data-
> >smu_buffer.mc_addr_low);
> + }
>   fw_to_load = UCODE_ID_RLC_G_MASK
>  + UCODE_ID_SDMA0_MASK
>  + UCODE_ID_SDMA1_MASK
> @@ -543,7 +549,6 @@ int smu7_init(struct pp_smumgr *smumgr)
>   smu_data = (struct smu7_smumgr *)(smumgr->backend);
>   smu_data->header_buffer.data_size =
>   ((sizeof(struct SMU_DRAMData_TOC) / 4096) + 1) *
> 4096;
> - smu_data->smu_buffer.data_size = 200*4096;

Was this change intended?  I'm assuming this is extraneous as it's set in 
smu7_init() as well.

Alex

> 
>  /* Allocate FW image data structure and header buffer and
>   * send the header buffer address to SMU */
> @@ -566,6 +571,10 @@ int smu7_init(struct pp_smumgr *smumgr)
>   (cgs_handle_t)smu_data->header_buffer.handle);
>   return -EINVAL);
> 
> + if (cgs_is_virtualization_enabled(smumgr->device))
> + return 0;
> +
> + smu_data->smu_buffer.data_size = 200*4096;
>   smu_allocate_memory(smumgr->device,
>   smu_data->smu_buffer.data_size,
>   CGS_GPU_MEM_TYPE__VISIBLE_CONTIG_FB,
> --
> 2.7.4
> 
> ___
> 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