All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

* [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

* 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.