Re: [PATCH 2/2] drm/amdgpu/soc15: always load the psp IP module

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 6913 bytes --]

That still leaves us with the issue that we need the PSP for the GPU 
reset independent if it is used for firmware loading or not.

Additional to that the PSP is present in the hardware no matter if we 
use it or not. So I think we should always at least add it.

Regards,
Christian.

Am 09.03.2018 um 18:53 schrieb Zhu, Rex:
>
>  >I also don't really like the fact that we use the module parameter 
> directly to determine whether to load the >PSP module or not, we 
> should be using adev->firmware.load_type, but that doesn't get set 
> until later.
>
>
> we can move function amdgpu_ucode_get_load_type to 
> amdgpu_check_arguments(adev);
> adev->firmware.load_type will be set.
>
> >The problem with checking the module parameter is that that param is 
> gobal so if you you have multiple >GPUs, you may get messed up.
>
> if user set the load type through module parameters, it is valid for 
> all gpu.
>
> Best Regards
> Rex
>
>
> ------------------------------------------------------------------------
> *From:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Sent:* Saturday, March 10, 2018 1:02 AM
> *To:* Koenig, Christian
> *Cc:* Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; Deucher, Alexander
> *Subject:* Re: [PATCH 2/2] drm/amdgpu/soc15: always load the psp IP 
> module
> On Fri, Mar 9, 2018 at 2:45 AM, Christian König 
> <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 
> <mailto:ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>
>     Hi Rex,
>
>     I think still initializing the PSP even when you don't need it for
>     firmware upload sounds like a good idea to me.
>
>     But take that with a grain of salt since I really on don't know
>     that part of the hardware so well.
>
>
> Right.  We need PSP for GPU resets among other things.  I also don't 
> really like the fact that we use the module parameter directly to 
> determine whether to load the PSP module or not, we should be using 
> adev->firmware.load_type, but that doesn't get set until later.  We 
> should probably just move that earlier in the common code rather than 
> having it in the soc files.  The problem with checking the module 
> parameter is that that param is gobal so if you you have multiple 
> GPUs, you may get messed up.
>
> Alex
>
>
>     Christian.
>
>
>     Am 09.03.2018 um 06:10 schrieb Zhu, Rex:
>>
>>     Hi Alex,
>>
>>
>>     How about keep the firmware type checking in set_ip_blocks.
>>
>>     and remove the same check code in psp module.
>>
>>     also no need to change load type if psp load firmware failed in
>>     psp module.
>>
>>
>>     Please review the attached patch.
>>
>>
>>     Best Regards
>>
>>     Rex
>>
>>     ------------------------------------------------------------------------
>>     *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>     <mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Alex
>>     Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> <mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>     *Sent:* Friday, March 9, 2018 4:54 AM
>>     *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>>     <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>     *Cc:* Deucher, Alexander
>>     *Subject:* [PATCH 2/2] drm/amdgpu/soc15: always load the psp IP
>>     module
>>     We already handle the firmware loading type checks in the
>>     psp module directly, no need for an additional check.
>>
>>     Signed-off-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
>>     <mailto:alexander.deucher-5C7GfCeVMHo@public.gmane.org>
>>     ---
>>      drivers/gpu/drm/amd/amdgpu/soc15.c | 3 +--
>>      1 file changed, 1 insertion(+), 2 deletions(-)
>>
>>     diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
>>     b/drivers/gpu/drm/amd/amdgpu/soc15.c
>>     index 8dc8b72ed49b..ecf58a68cf66 100644
>>     --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
>>     +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
>>     @@ -531,8 +531,7 @@ int soc15_set_ip_blocks(struct amdgpu_device
>>     *adev)
>>     amdgpu_device_ip_block_add(adev, &vega10_common_ip_block);
>>     amdgpu_device_ip_block_add(adev, &gmc_v9_0_ip_block);
>>     amdgpu_device_ip_block_add(adev, &vega10_ih_ip_block);
>>     -               if (amdgpu_fw_load_type == 2 ||
>>     amdgpu_fw_load_type == -1)
>>     - amdgpu_device_ip_block_add(adev, &psp_v3_1_ip_block);
>>     + amdgpu_device_ip_block_add(adev, &psp_v3_1_ip_block);
>>                      if (!amdgpu_sriov_vf(adev))
>>     amdgpu_device_ip_block_add(adev, &amdgpu_pp_ip_block);
>>                      if (adev->enable_virtual_display ||
>>     amdgpu_sriov_vf(adev))
>>     -- 
>>     2.13.6
>>
>>     _______________________________________________
>>     amd-gfx mailing list
>>     amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>     https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>     <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>>     amd-gfx Info Page - freedesktop.org
>>     <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>>     lists.freedesktop.org
>>     Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
>>     following form. Use of all freedesktop.org lists is subject to
>>     our Code of ...
>>
>>
>>     amd-gfx Info Page - freedesktop.org
>>     <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>>     lists.freedesktop.org <http://lists.freedesktop.org>
>>     Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
>>     following form. Use of all freedesktop.org
>>     <http://freedesktop.org> lists is subject to our Code of ...
>>     www <http://freedesktop.org/>
>>     freedesktop.org
>>     Welcome to freedesktop.org. freedesktop.org is open source / open
>>     discussion software projects working on interoperability and
>>     shared technology for X Window System ...
>>
>>
>>
>>
>>
>>     _______________________________________________
>>     amd-gfx mailing list
>>     amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>     https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>     <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>>     amd-gfx Info Page - freedesktop.org
>>     <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
>>     lists.freedesktop.org
>>     Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the
>>     following form. Use of all freedesktop.org lists is subject to
>>     our Code of ...
>>
>>
>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 45944 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx