Re: [PATCH v2 1/2] drm/amd/display: move remaining FPU code to dml folder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Christian,

thank you for your review. I got a warning in checking the first patch with checkpatch.pl.

I'll fix it in the next version.

---------------------------------------------------------------
0001-drm-amd-display-move-remaining-FPU-code-to-dml-folde.patch
---------------------------------------------------------------
WARNING:braces {} are not necessary for single statement blocks
#131: FILE: drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.c:157:
+               if (ASICREV_IS_RV1_F0(dc->ctx->asic_id.hw_internal_rev)) {
+                       dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 = 20.80f;
+               }

total: 0 errors, 1 warnings, 110 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
     mechanically convert to the typical style using --fix or --fix-inplace.

0001-drm-amd-display-move-remaining-FPU-code-to-dml-folde.patch has style problems, please review.
---------------------------------------------------------------
0002-drm-amd-display-move-remaining-FPU-code-to-dml-folde.patch
---------------------------------------------------------------
total: 0 errors, 0 warnings, 29 lines checked

0002-drm-amd-display-move-remaining-FPU-code-to-dml-folde.patch has no obvious style problems and is ready for submission.

NOTE: If any of the errors are false positives, please report
     them to the maintainer, see CHECKPATCH in MAINTAINERS.

Am 26.10.22 um 09:19 schrieb Christian König:
> Am 25.10.22 um 23:17 schrieb Ao Zhong:
>> In the process of enabling DCN support for arm64, I found that the
>> dcn10_resource_construct_fp function in dcn10/dcn10_resource.c still
>> needs to use FPU. This will cause compilation to fail on ARM64 platforms
>> because -mgeneral-regs-only is enabled by default to disable the
>> hardware FPU. So move dcn10_resource_construct_fp from dcn10 folder to
>> dml/dcn10 folder to enable hardware FPU for that function.
>
> Of hand that looks good to me, but our display team needs to take a look.
>
> Feel free to add an Acked-by: Christian König <christian.koenig@xxxxxxx> for the series.
>
> While at it could you make sure that checkpatch.pl doesn't has anything to complain about the moved code?
>
> Thanks for the help,
> Christian.
>
>>
>> Signed-off-by: Ao Zhong <hacc1225@xxxxxxxxx>
>> ---
>>   .../drm/amd/display/dc/dcn10/dcn10_resource.c | 44 +------------------
>>   .../drm/amd/display/dc/dml/dcn10/dcn10_fpu.c  | 38 ++++++++++++++++
>>   .../drm/amd/display/dc/dml/dcn10/dcn10_fpu.h  |  2 +
>>   3 files changed, 42 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> index 56d30baf12df..6bfac8088ab0 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> @@ -1295,47 +1295,6 @@ static uint32_t read_pipe_fuses(struct dc_context *ctx)
>>       return value;
>>   }
>>   -/*
>> - * Some architectures don't support soft-float (e.g. aarch64), on those
>> - * this function has to be called with hardfloat enabled, make sure not
>> - * to inline it so whatever fp stuff is done stays inside
>> - */
>> -static noinline void dcn10_resource_construct_fp(
>> -    struct dc *dc)
>> -{
>> -    if (dc->ctx->dce_version == DCN_VERSION_1_01) {
>> -        struct dcn_soc_bounding_box *dcn_soc = dc->dcn_soc;
>> -        struct dcn_ip_params *dcn_ip = dc->dcn_ip;
>> -        struct display_mode_lib *dml = &dc->dml;
>> -
>> -        dml->ip.max_num_dpp = 3;
>> -        /* TODO how to handle 23.84? */
>> -        dcn_soc->dram_clock_change_latency = 23;
>> -        dcn_ip->max_num_dpp = 3;
>> -    }
>> -    if (ASICREV_IS_RV1_F0(dc->ctx->asic_id.hw_internal_rev)) {
>> -        dc->dcn_soc->urgent_latency = 3;
>> -        dc->debug.disable_dmcu = true;
>> -        dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 = 41.60f;
>> -    }
>> -
>> -
>> -    dc->dcn_soc->number_of_channels = dc->ctx->asic_id.vram_width / ddr4_dram_width;
>> -    ASSERT(dc->dcn_soc->number_of_channels < 3);
>> -    if (dc->dcn_soc->number_of_channels == 0)/*old sbios bug*/
>> -        dc->dcn_soc->number_of_channels = 2;
>> -
>> -    if (dc->dcn_soc->number_of_channels == 1) {
>> -        dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 = 19.2f;
>> -        dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 = 17.066f;
>> -        dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 = 14.933f;
>> -        dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 = 12.8f;
>> -        if (ASICREV_IS_RV1_F0(dc->ctx->asic_id.hw_internal_rev)) {
>> -            dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 = 20.80f;
>> -        }
>> -    }
>> -}
>> -
>>   static bool verify_clock_values(struct dm_pp_clock_levels_with_voltage *clks)
>>   {
>>       int i;
>> @@ -1510,8 +1469,9 @@ static bool dcn10_resource_construct(
>>       memcpy(dc->dcn_ip, &dcn10_ip_defaults, sizeof(dcn10_ip_defaults));
>>       memcpy(dc->dcn_soc, &dcn10_soc_defaults, sizeof(dcn10_soc_defaults));
>>   -    /* Other architectures we build for build this with soft-float */
>> +    DC_FP_START();
>>       dcn10_resource_construct_fp(dc);
>> +    DC_FP_END();
>>         if (!dc->config.is_vmin_only_asic)
>>           if (ASICREV_IS_RAVEN2(dc->ctx->asic_id.hw_internal_rev))
>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.c
>> index 99644d896222..8b5e6fff5444 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.c
>> @@ -27,6 +27,8 @@
>>   #include "dcn10/dcn10_resource.h"
>>     #include "dcn10_fpu.h"
>> +#include "resource.h"
>> +#include "amdgpu_dm/dc_fpu.h"
>>     /**
>>    * DOC: DCN10 FPU manipulation Overview
>> @@ -121,3 +123,39 @@ struct _vcs_dpi_soc_bounding_box_st dcn1_0_soc = {
>>       .writeback_dram_clock_change_latency_us = 23.0,
>>       .return_bus_width_bytes = 64,
>>   };
>> +
>> +void dcn10_resource_construct_fp(struct dc *dc)
>> +{
>> +    dc_assert_fp_enabled();
>> +    if (dc->ctx->dce_version == DCN_VERSION_1_01) {
>> +        struct dcn_soc_bounding_box *dcn_soc = dc->dcn_soc;
>> +        struct dcn_ip_params *dcn_ip = dc->dcn_ip;
>> +        struct display_mode_lib *dml = &dc->dml;
>> +
>> +        dml->ip.max_num_dpp = 3;
>> +        /* TODO how to handle 23.84? */
>> +        dcn_soc->dram_clock_change_latency = 23;
>> +        dcn_ip->max_num_dpp = 3;
>> +    }
>> +    if (ASICREV_IS_RV1_F0(dc->ctx->asic_id.hw_internal_rev)) {
>> +        dc->dcn_soc->urgent_latency = 3;
>> +        dc->debug.disable_dmcu = true;
>> +        dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 = 41.60f;
>> +    }
>> +
>> +
>> +    dc->dcn_soc->number_of_channels = dc->ctx->asic_id.vram_width / ddr4_dram_width;
>> +    ASSERT(dc->dcn_soc->number_of_channels < 3);
>> +    if (dc->dcn_soc->number_of_channels == 0)/*old sbios bug*/
>> +        dc->dcn_soc->number_of_channels = 2;
>> +
>> +    if (dc->dcn_soc->number_of_channels == 1) {
>> +        dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 = 19.2f;
>> +        dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8 = 17.066f;
>> +        dc->dcn_soc->fabric_and_dram_bandwidth_vmid0p72 = 14.933f;
>> +        dc->dcn_soc->fabric_and_dram_bandwidth_vmin0p65 = 12.8f;
>> +        if (ASICREV_IS_RV1_F0(dc->ctx->asic_id.hw_internal_rev)) {
>> +            dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9 = 20.80f;
>> +        }
>> +    }
>> +}
>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h
>> index e74ed4b4ce5b..63219ecd8478 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn10/dcn10_fpu.h
>> @@ -27,4 +27,6 @@
>>   #ifndef __DCN10_FPU_H__
>>   #define __DCN10_FPU_H__
>>   +void dcn10_resource_construct_fp(struct dc *dc);
>> +
>>   #endif /* __DCN20_FPU_H__ */
>



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux