amdgpu/dc: use kernel ilog2 for log_2.
Subsystems (3)
PATCH 1/4 amdgpu/dc: use kernel ilog2 for log_2.
From: Dave Airlie <[email protected]>
This should produce the same result.
--- drivers/gpu/drm/amd/display/dc/basics/conversion.c | 10 ---------- drivers/gpu/drm/amd/display/dc/basics/conversion.h | 5 ++++- 2 files changed, 4 insertions(+), 11 deletions(-)
drivers/gpu/drm/amd/display/dc/basics/conversion.c +0 -10
@@ -102,13 +102,3 @@ void convert_float_matrix( matrix[i] = (uint16_t )reg_value; } } - -unsigned int log_2 (unsigned int num) -{ -unsigned int result =0 ; - -while ((num >>=1 ) !=0 ) - result++; - -return result; -}
drivers/gpu/drm/amd/display/dc/basics/conversion.h +4 -1
@@ -38,6 +38,9 @@ void convert_float_matrix(struct fixed31_32 *flt, uint32_t buffer_size); -unsigned int log_2 (unsigned int num); +static inline unsigned int log_2 (unsigned int num) +{ +return ilog2 (num); +}#endif
Signature · 6 lines
2.9.5 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 2/4 amdgpu/dc: drop dce110_types.h
In reply to a message not in the archiveFrom: Dave Airlie <[email protected]>
Doesn't appear to be used.
--- .../gpu/drm/amd/display/dc/dce110/dce110_types.h | 30 ---------------------- 1 file changed, 30 deletions(-) delete mode 100644 drivers/gpu/drm/amd/display/dc/dce110/dce110_types.h
drivers/gpu/drm/amd/display/dc/dce110/dce110_types.h deleted +0 -30
@@ -1,30 +0,0 @@ -/* Copyright 2012-15 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. -* -* Authors: AMD -* -*/ - -#ifndef _DCE110_TYPES_H_ -#define __DCE110_TYPES_H_ - -#define GAMMA_SEGMENTS_NUM 16 - -#endif /* DRIVERS_GPU_DRM_AMD_DC_DEV_DC_DCE110_DCE110_TYPES_H_ */
Signature · 6 lines
2.9.5 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 3/4 amdgpu/dc: drop hw_sequencer_types.h
In reply to a message not in the archiveFrom: Dave Airlie <[email protected]>
This isn't used or required.
--- .../display/dc/dce110/dce110_timing_generator.h | 1 - .../display/dc/dce120/dce120_timing_generator.h | 1 - drivers/gpu/drm/amd/display/dc/inc/core_types.h | 1 - .../gpu/drm/amd/display/dc/inc/hw/stream_encoder.h | 1 - .../drm/amd/display/include/hw_sequencer_types.h | 33 ---------------------- 5 files changed, 37 deletions(-) delete mode 100644 drivers/gpu/drm/amd/display/include/hw_sequencer_types.h
drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.h +0 -1
@@ -28,7 +28,6 @@#include "timing_generator.h" #include "../include/grph_object_id.h" -#include "../include/hw_sequencer_types.h" /* GSL Sync related values */
drivers/gpu/drm/amd/display/dc/dce120/dce120_timing_generator.h +0 -1
@@ -28,7 +28,6 @@#include "timing_generator.h" #include "../include/grph_object_id.h" -#include "../include/hw_sequencer_types.h" #include "dce110/dce110_timing_generator.h"
drivers/gpu/drm/amd/display/dc/inc/core_types.h +0 -1
@@ -46,7 +46,6 @@ void enable_surface_flip_reporting(struct dc_plane_state *plane_state,#include "stream_encoder.h" #include "clock_source.h" #include "audio.h" -#include "hw_sequencer_types.h" #include "dm_pp_smu.h"
drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h +0 -1
@@ -6,7 +6,6 @@#ifndef STREAM_ENCODER_H_ #define STREAM_ENCODER_H_ -#include "include/hw_sequencer_types.h" #include "audio_types.h" struct dc_bios ;
drivers/gpu/drm/amd/display/include/hw_sequencer_types.h deleted +0 -33
@@ -1,33 +0,0 @@ -/* -* Copyright 2012-15 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. -* -* Authors: AMD -* -*/ - -#ifndef __DAL_HW_SEQUENCER_TYPES_H__ -#define __DAL_HW_SEQUENCER_TYPES_H__ - -#include "signal_types.h" -#include "grph_object_defs.h" -#include "link_service_types.h" - -#endif
Signature · 6 lines
2.9.5 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
PATCH 4/4 amdgpu/dc: remove bitmap implementation in gpio_service
In reply to a message not in the archiveFrom: Dave Airlie <[email protected]>
This handrolls a bit map implementation (linux/bitmap.h), but it also actually doesn't need it, the max value greppable in the code is 31 for a gpio count. So just use a uint32_t for now.
This should probably migrate to using the linux/bitops.h operations, but for now just rip out the bitmap implementation and fail if we
Quoted 1 line
32 bits.
--- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 85 +++------------------- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h | 8 +- 2 files changed, 11 insertions(+), 82 deletions(-)
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c +10 -75
@@ -56,8 +56,7 @@ struct gpio_service *dal_gpio_service_create( struct dc_context *ctx) {struct gpio_service *service; - -uint32_t index_of_id; +int i; service =kzalloc (sizeof (struct gpio_service ),GFP_KERNEL );
@@ -78,62 +77,16 @@ struct gpio_service *dal_gpio_service_create( goto failure_1; } -/* allocate and initialize business storage */ - { -const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - - index_of_id =0 ; - service->ctx = ctx; - -do { -uint32_t number_of_bits = - service->factory.number_of_pins[index_of_id]; - -uint32_t number_of_uints = - (number_of_bits + bits_per_uint -1 ) / - bits_per_uint; - -uint32_t *slot; - -if (number_of_bits) { -uint32_t index_of_uint =0 ; + service->ctx = ctx; - slot =kzalloc (number_of_uints *sizeof (uint32_t ), -GFP_KERNEL ); - -if (!slot) { -BREAK_TO_DEBUGGER (); - goto failure_2; - } - -do { - slot[index_of_uint] =0 ; - - ++index_of_uint; - }while (index_of_uint < number_of_uints); - }else - slot =NULL ; - - service->busyness[index_of_id] = slot; - - ++index_of_id; - }while (index_of_id <GPIO_ID_COUNT ); +for (i =0 ; i <GPIO_ID_COUNT ; i++) { +if (service->factory.number_of_pins[i] >32 ) { +BREAK_TO_DEBUGGER (); + goto failure_1; + } } -return service; -failure_2: -while (index_of_id) { -uint32_t *slot; - - --index_of_id; - - slot = service->busyness[index_of_id]; - -if (slot) -kfree (slot); - }; - failure_1:kfree (service);
@@ -164,20 +117,6 @@ void dal_gpio_service_destroy(return ; } -/* free business storage */ - { -uint32_t index_of_id =0 ; - -do { -uint32_t *slot = (*ptr)->busyness[index_of_id]; - -if (slot) -kfree (slot); - - ++index_of_id; - }while (index_of_id <GPIO_ID_COUNT ); - } -kfree (*ptr); *ptr =NULL ;
@@ -195,9 +134,7 @@ static bool is_pin_busy( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; -const uint32_t *slot = service->busyness[id] + (en / bits_per_uint); - -return 0 != (*slot & (1 << (en % bits_per_uint))); +return 0 != (service->busyness[id] & (1 << (en % bits_per_uint))); } static void set_pin_busy(
@@ -207,8 +144,7 @@ static void set_pin_busy( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - service->busyness[id][en / bits_per_uint] |= - (1 << (en % bits_per_uint)); + service->busyness[id] |= (1 << (en % bits_per_uint)); } static void set_pin_free(
@@ -218,8 +154,7 @@ static void set_pin_free( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - service->busyness[id][en / bits_per_uint] &= - ~(1 << (en % bits_per_uint)); + service->busyness[id] &= ~(1 << (en % bits_per_uint)); } enum gpio_result dal_gpio_service_open(
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h +1 -7
@@ -33,13 +33,7 @@ struct gpio_service {struct dc_context *ctx;struct hw_translate translate;struct hw_factory factory; -/* -* @brief -* Business storage. -* For each member of 'enum gpio_id', -* store array of bits (packed into uint32_t slots), -* index individual bit by 'en' value */ -uint32_t *busyness[GPIO_ID_COUNT ]; +uint32_t busyness[GPIO_ID_COUNT ]; }; enum gpio_result dal_gpio_service_open(
Signature · 6 lines
2.9.5 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Harry Wentland
Reviewed-byRe: [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service
In reply to a message not in the archiveQuoted 11 lines · Dave Airlie
From: Dave Airlie <[email protected]>
This handrolls a bit map implementation (
linux/bitmap.h), but it also actually doesn't need it, the max value greppable in the code is 31 for a gpio count. So just use auint32_tfor now.This should probably migrate to using the
linux/bitops.hoperations, but for now just rip out the bitmap implementation and fail if weQuoted 1 line
32 bits.
Signed-off-by Dave Airlie <[email protected]>
Nice cleanup. This was such overengineered code.
This series is
Looks like you sent the ilog2 patch twice. I'll ignore the separate one as it seems to be the same as the one from this series.
Harry
Quoted 150 lines
--- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 85 +++------------------- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h | 8 +- 2 files changed, 11 insertions(+), 82 deletions(-)drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c +10
-75@@ -56,8 +56,7 @@ struct gpio_service *dal_gpio_service_create( struct dc_context *ctx) {struct gpio_service *service; - -uint32_t index_of_id; +int i; service =kzalloc (sizeof (struct gpio_service ),GFP_KERNEL );@@ -78,62 +77,16 @@ struct gpio_service *dal_gpio_service_create( goto failure_1; } -/* allocate and initialize business storage */ - { -const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - - index_of_id =0 ; - service->ctx = ctx; - -do { -uint32_t number_of_bits = - service->factory.number_of_pins[index_of_id]; - -uint32_t number_of_uints = - (number_of_bits + bits_per_uint -1 ) / - bits_per_uint; - -uint32_t *slot; - -if (number_of_bits) { -uint32_t index_of_uint =0 ; + service->ctx = ctx; - slot =kzalloc (number_of_uints *sizeof (uint32_t ), -GFP_KERNEL ); - -if (!slot) { -BREAK_TO_DEBUGGER (); - goto failure_2; - } - -do { - slot[index_of_uint] =0 ; - - ++index_of_uint; - }while (index_of_uint < number_of_uints); - }else - slot =NULL ; - - service->busyness[index_of_id] = slot; - - ++index_of_id; - }while (index_of_id <GPIO_ID_COUNT ); +for (i =0 ; i <GPIO_ID_COUNT ; i++) { +if (service->factory.number_of_pins[i] >32 ) { +BREAK_TO_DEBUGGER (); + goto failure_1; + } } -return service; -failure_2: -while (index_of_id) { -uint32_t *slot; - - --index_of_id; - - slot = service->busyness[index_of_id]; - -if (slot) -kfree (slot); - }; - failure_1:kfree (service);@@ -164,20 +117,6 @@ void dal_gpio_service_destroy(return ; } -/* free business storage */ - { -uint32_t index_of_id =0 ; - -do { -uint32_t *slot = (*ptr)->busyness[index_of_id]; - -if (slot) -kfree (slot); - - ++index_of_id; - }while (index_of_id <GPIO_ID_COUNT ); - } -kfree (*ptr); *ptr =NULL ;@@ -195,9 +134,7 @@ static bool is_pin_busy( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; -const uint32_t *slot = service->busyness[id] + (en / bits_per_uint); - -return 0 != (*slot & (1 << (en % bits_per_uint))); +return 0 != (service->busyness[id] & (1 << (en % bits_per_uint))); } static void set_pin_busy(@@ -207,8 +144,7 @@ static void set_pin_busy( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - service->busyness[id][en / bits_per_uint] |= - (1 << (en % bits_per_uint)); + service->busyness[id] |= (1 << (en % bits_per_uint)); } static void set_pin_free(@@ -218,8 +154,7 @@ static void set_pin_free( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - service->busyness[id][en / bits_per_uint] &= - ~(1 << (en % bits_per_uint)); + service->busyness[id] &= ~(1 << (en % bits_per_uint)); } enum gpio_result dal_gpio_service_open(drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h +0
-1@@ -33,13 +33,7 @@ struct gpio_service {struct dc_context *ctx;struct hw_translate translate;struct hw_factory factory; -/*
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h struct gpio_service { · line 39
Quoted 10 lines
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h +1
-6-* @brief -* Business storage. -* For each member of 'enum gpio_id', -* store array of bits (packed into uint32_t slots), -* index individual bit by 'en' value */ -uint32_t *busyness[GPIO_ID_COUNT ]; +uint32_t busyness[GPIO_ID_COUNT ]; }; enum gpio_result dal_gpio_service_open(
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service
In reply to a message not in the archiveQuoted 8 lines · Dave Airlie
From: Dave Airlie <[email protected]>
This handrolls a bit map implementation (
linux/bitmap.h), but it also actually doesn't need it, the max value greppable in the code is 31 for a gpio count. So just use auint32_tfor now.This should probably migrate to using the
linux/bitops.hoperations, but for now just rip out the bitmap implementation and fail if we
I think bitops are atomic, which may not be needed here. We've also run into issues with bitops on other CPU architectures with KFD, because they're 64-bit ops that can fail with incorrect alignment. We ended up converting some of them to shifts and masks for that reason.
Regards,
Felix
Quoted 151 lines
Quoted 1 line
32 bits.
Signed-off-by Dave Airlie <[email protected]>--- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 85 +++------------------- drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h | 8 +- 2 files changed, 11 insertions(+), 82 deletions(-)drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c +10
-75@@ -56,8 +56,7 @@ struct gpio_service *dal_gpio_service_create( struct dc_context *ctx) {struct gpio_service *service; - -uint32_t index_of_id; +int i; service =kzalloc (sizeof (struct gpio_service ),GFP_KERNEL );@@ -78,62 +77,16 @@ struct gpio_service *dal_gpio_service_create( goto failure_1; } -/* allocate and initialize business storage */ - { -const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - - index_of_id =0 ; - service->ctx = ctx; - -do { -uint32_t number_of_bits = - service->factory.number_of_pins[index_of_id]; - -uint32_t number_of_uints = - (number_of_bits + bits_per_uint -1 ) / - bits_per_uint; - -uint32_t *slot; - -if (number_of_bits) { -uint32_t index_of_uint =0 ; + service->ctx = ctx; - slot =kzalloc (number_of_uints *sizeof (uint32_t ), -GFP_KERNEL ); - -if (!slot) { -BREAK_TO_DEBUGGER (); - goto failure_2; - } - -do { - slot[index_of_uint] =0 ; - - ++index_of_uint; - }while (index_of_uint < number_of_uints); - }else - slot =NULL ; - - service->busyness[index_of_id] = slot; - - ++index_of_id; - }while (index_of_id <GPIO_ID_COUNT ); +for (i =0 ; i <GPIO_ID_COUNT ; i++) { +if (service->factory.number_of_pins[i] >32 ) { +BREAK_TO_DEBUGGER (); + goto failure_1; + } } -return service; -failure_2: -while (index_of_id) { -uint32_t *slot; - - --index_of_id; - - slot = service->busyness[index_of_id]; - -if (slot) -kfree (slot); - }; - failure_1:kfree (service);@@ -164,20 +117,6 @@ void dal_gpio_service_destroy(return ; } -/* free business storage */ - { -uint32_t index_of_id =0 ; - -do { -uint32_t *slot = (*ptr)->busyness[index_of_id]; - -if (slot) -kfree (slot); - - ++index_of_id; - }while (index_of_id <GPIO_ID_COUNT ); - } -kfree (*ptr); *ptr =NULL ;@@ -195,9 +134,7 @@ static bool is_pin_busy( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; -const uint32_t *slot = service->busyness[id] + (en / bits_per_uint); - -return 0 != (*slot & (1 << (en % bits_per_uint))); +return 0 != (service->busyness[id] & (1 << (en % bits_per_uint))); } static void set_pin_busy(@@ -207,8 +144,7 @@ static void set_pin_busy( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - service->busyness[id][en / bits_per_uint] |= - (1 << (en % bits_per_uint)); + service->busyness[id] |= (1 << (en % bits_per_uint)); } static void set_pin_free(@@ -218,8 +154,7 @@ static void set_pin_free( {const uint32_t bits_per_uint =sizeof (uint32_t ) <<3 ; - service->busyness[id][en / bits_per_uint] &= - ~(1 << (en % bits_per_uint)); + service->busyness[id] &= ~(1 << (en % bits_per_uint)); } enum gpio_result dal_gpio_service_open(drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h +0
-1@@ -33,13 +33,7 @@ struct gpio_service {struct dc_context *ctx;struct hw_translate translate;struct hw_factory factory; -/*
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h struct gpio_service { · line 39
Quoted 10 lines
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h +1
-6-* @brief -* Business storage. -* For each member of 'enum gpio_id', -* store array of bits (packed into uint32_t slots), -* index individual bit by 'en' value */ -uint32_t *busyness[GPIO_ID_COUNT ]; +uint32_t busyness[GPIO_ID_COUNT ]; }; enum gpio_result dal_gpio_service_open(
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx