* [PATCH 1/4] amdgpu/dc: use kernel ilog2 for log_2.
@ 2017-10-03 3:49 Dave Airlie
[not found] ` <20171003034945.27909-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Dave Airlie @ 2017-10-03 3:49 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Dave Airlie <airlied@redhat.com>
This should produce the same result.
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
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(-)
diff --git a/drivers/gpu/drm/amd/display/dc/basics/conversion.c b/drivers/gpu/drm/amd/display/dc/basics/conversion.c
index a2e22ae..23c9a0e 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/conversion.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/conversion.c
@@ -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;
-}
diff --git a/drivers/gpu/drm/amd/display/dc/basics/conversion.h b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
index 189325f..ade785c 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/conversion.h
+++ b/drivers/gpu/drm/amd/display/dc/basics/conversion.h
@@ -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
--
2.9.5
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread[parent not found: <20171003034945.27909-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH 2/4] amdgpu/dc: drop dce110_types.h [not found] ` <20171003034945.27909-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2017-10-03 3:49 ` Dave Airlie 2017-10-03 3:49 ` [PATCH 3/4] amdgpu/dc: drop hw_sequencer_types.h Dave Airlie 2017-10-03 3:49 ` [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service Dave Airlie 2 siblings, 0 replies; 6+ messages in thread From: Dave Airlie @ 2017-10-03 3:49 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW From: Dave Airlie <airlied@redhat.com> Doesn't appear to be used. Signed-off-by: Dave Airlie <airlied@redhat.com> --- .../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 diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_types.h b/drivers/gpu/drm/amd/display/dc/dce110/dce110_types.h deleted file mode 100644 index 55f5238..0000000 --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_types.h +++ /dev/null @@ -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_ */ -- 2.9.5 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] amdgpu/dc: drop hw_sequencer_types.h [not found] ` <20171003034945.27909-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-10-03 3:49 ` [PATCH 2/4] amdgpu/dc: drop dce110_types.h Dave Airlie @ 2017-10-03 3:49 ` Dave Airlie 2017-10-03 3:49 ` [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service Dave Airlie 2 siblings, 0 replies; 6+ messages in thread From: Dave Airlie @ 2017-10-03 3:49 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW From: Dave Airlie <airlied@redhat.com> This isn't used or required. Signed-off-by: Dave Airlie <airlied@redhat.com> --- .../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 diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.h b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.h index bd8d0ab..82737de 100644 --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.h +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.h @@ -28,7 +28,6 @@ #include "timing_generator.h" #include "../include/grph_object_id.h" -#include "../include/hw_sequencer_types.h" /* GSL Sync related values */ diff --git a/drivers/gpu/drm/amd/display/dc/dce120/dce120_timing_generator.h b/drivers/gpu/drm/amd/display/dc/dce120/dce120_timing_generator.h index d69871e..549d70b 100644 --- a/drivers/gpu/drm/amd/display/dc/dce120/dce120_timing_generator.h +++ b/drivers/gpu/drm/amd/display/dc/dce120/dce120_timing_generator.h @@ -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" diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h index d786ec4..ff23f26 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h +++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h @@ -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" diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h index 6ff90a0f..3050afe 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h @@ -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; diff --git a/drivers/gpu/drm/amd/display/include/hw_sequencer_types.h b/drivers/gpu/drm/amd/display/include/hw_sequencer_types.h deleted file mode 100644 index 8ba9f65..0000000 --- a/drivers/gpu/drm/amd/display/include/hw_sequencer_types.h +++ /dev/null @@ -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 -- 2.9.5 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service [not found] ` <20171003034945.27909-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-10-03 3:49 ` [PATCH 2/4] amdgpu/dc: drop dce110_types.h Dave Airlie 2017-10-03 3:49 ` [PATCH 3/4] amdgpu/dc: drop hw_sequencer_types.h Dave Airlie @ 2017-10-03 3:49 ` Dave Airlie [not found] ` <20171003034945.27909-4-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2 siblings, 1 reply; 6+ messages in thread From: Dave Airlie @ 2017-10-03 3:49 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW From: Dave Airlie <airlied@redhat.com> 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 > 32 bits. Signed-off-by: Dave Airlie <airlied@redhat.com> --- 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(-) diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c index d4e5ef6..95df7d4 100644 --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c @@ -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( diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h index c7f3081..96c52be 100644 --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h @@ -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( -- 2.9.5 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <20171003034945.27909-4-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service [not found] ` <20171003034945.27909-4-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2017-10-03 16:10 ` Harry Wentland 2017-10-03 19:29 ` Felix Kuehling 1 sibling, 0 replies; 6+ messages in thread From: Harry Wentland @ 2017-10-03 16:10 UTC (permalink / raw) To: Dave Airlie, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2017-10-02 11:49 PM, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > 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 >> 32 bits. > > Signed-off-by: Dave Airlie <airlied@redhat.com> Nice cleanup. This was such overengineered code. This series is Reviewed-by: Harry Wentland <harry.wentland@amd.com> 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 > --- > 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(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c > index d4e5ef6..95df7d4 100644 > --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c > +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c > @@ -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( > diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h > index c7f3081..96c52be 100644 > --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h > +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h > @@ -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( > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service [not found] ` <20171003034945.27909-4-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-10-03 16:10 ` Harry Wentland @ 2017-10-03 19:29 ` Felix Kuehling 1 sibling, 0 replies; 6+ messages in thread From: Felix Kuehling @ 2017-10-03 19:29 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2017-10-02 11:49 PM, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > 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 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 >> 32 bits. > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > 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(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c > index d4e5ef6..95df7d4 100644 > --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c > +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c > @@ -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( > diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h > index c7f3081..96c52be 100644 > --- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h > +++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h > @@ -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( _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-03 19:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-03 3:49 [PATCH 1/4] amdgpu/dc: use kernel ilog2 for log_2 Dave Airlie
[not found] ` <20171003034945.27909-1-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-03 3:49 ` [PATCH 2/4] amdgpu/dc: drop dce110_types.h Dave Airlie
2017-10-03 3:49 ` [PATCH 3/4] amdgpu/dc: drop hw_sequencer_types.h Dave Airlie
2017-10-03 3:49 ` [PATCH 4/4] amdgpu/dc: remove bitmap implementation in gpio_service Dave Airlie
[not found] ` <20171003034945.27909-4-airlied-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-03 16:10 ` Harry Wentland
2017-10-03 19:29 ` Felix Kuehling
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.