[PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
Christian König
ckoenig.leichtzumerken at gmail.com
Fri Apr 17 09:22:40 UTC 2020
Agreed, just wanted to reply as well since I think something is not
correctly understood here.
The cpu_to_be16() and be16_to_cpu() functions work different depending
on which architecture/endianess your are.
So they should be a NO-OP on x86 if everything is done right.
Christian.
Am 17.04.20 um 04:32 schrieb Deucher, Alexander:
>
> [AMD Public Use]
>
>
> I would drop this patch unless it only applies to APUs. On Linux,
> people may run the driver on big endian systems.
>
> Alex
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of
> Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> *Sent:* Thursday, April 16, 2020 7:40 PM
> *To:* amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
> *Cc:* Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Wentland, Harry
> <Harry.Wentland at amd.com>; Siqueira, Rodrigo
> <Rodrigo.Siqueira at amd.com>; Wood, Wyatt <Wyatt.Wood at amd.com>; Lakha,
> Bhawanpreet <Bhawanpreet.Lakha at amd.com>; Koo, Anthony
> <Anthony.Koo at amd.com>
> *Subject:* [PATCH 05/35] drm/amd/display: Remove byte swapping for
> dmcub abm config table
> From: Wyatt Wood <wyatt.wood at amd.com>
>
> [Why]
> Since x86 and dmcub are both little endian, byte swapping isn't
> necessary. Dmcu requires byte swapping as it is big endian.
>
> [How]
> Add flag to function definitions to determine if byte swapping is
> necessary.
>
> Signed-off-by: Wyatt Wood <wyatt.wood at amd.com>
> Reviewed-by: Anthony Koo <Anthony.Koo at amd.com>
> Acked-by: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> ---
> .../amd/display/modules/power/power_helpers.c | 74 +++++++++----------
> 1 file changed, 36 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
> b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
> index dd1517684c90..edb446455f6b 100644
> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
> @@ -240,7 +240,7 @@ static void fill_backlight_transform_table(struct
> dmcu_iram_parameters params,
> }
>
> static void fill_backlight_transform_table_v_2_2(struct
> dmcu_iram_parameters params,
> - struct iram_table_v_2_2 *table)
> + struct iram_table_v_2_2 *table, bool big_endian)
> {
> unsigned int i;
> unsigned int num_entries = NUM_BL_CURVE_SEGS;
> @@ -264,10 +264,10 @@ static void
> fill_backlight_transform_table_v_2_2(struct dmcu_iram_parameters par
> lut_index = (params.backlight_lut_array_size - 1) * i
> / (num_entries - 1);
> ASSERT(lut_index < params.backlight_lut_array_size);
>
> - table->backlight_thresholds[i] =
> - cpu_to_be16(DIV_ROUNDUP((i * 65536),
> num_entries));
> - table->backlight_offsets[i] =
> - cpu_to_be16(params.backlight_lut_array[lut_index]);
> + table->backlight_thresholds[i] = (big_endian) ?
> + cpu_to_be16(DIV_ROUNDUP((i * 65536),
> num_entries)) : DIV_ROUNDUP((i * 65536), num_entries);
> + table->backlight_offsets[i] = (big_endian) ?
> + cpu_to_be16(params.backlight_lut_array[lut_index]) :
> params.backlight_lut_array[lut_index];
> }
> }
>
> @@ -587,18 +587,16 @@ void fill_iram_v_2_2(struct iram_table_v_2_2
> *ram_table, struct dmcu_iram_parame
> ram_table->crgb_slope[7] = cpu_to_be16(0x1910);
>
> fill_backlight_transform_table_v_2_2(
> - params, ram_table);
> + params, ram_table, true);
> }
>
> -void fill_iram_v_2_3(struct iram_table_v_2_2 *ram_table, struct
> dmcu_iram_parameters params)
> +void fill_iram_v_2_3(struct iram_table_v_2_2 *ram_table, struct
> dmcu_iram_parameters params, bool big_endian)
> {
> unsigned int i, j;
> unsigned int set = params.set;
>
> ram_table->flags = 0x0;
> -
> - ram_table->min_abm_backlight =
> - cpu_to_be16(params.min_abm_backlight);
> + ram_table->min_abm_backlight = (big_endian) ?
> cpu_to_be16(params.min_abm_backlight) : params.min_abm_backlight;
>
> for (i = 0; i < NUM_AGGR_LEVEL; i++) {
> ram_table->hybrid_factor[i] =
> abm_settings[set][i].brightness_gain;
> @@ -622,33 +620,33 @@ void fill_iram_v_2_3(struct iram_table_v_2_2
> *ram_table, struct dmcu_iram_parame
> ram_table->iir_curve[4] = 0x65;
>
> //Gamma 2.2
> - ram_table->crgb_thresh[0] = cpu_to_be16(0x127c);
> - ram_table->crgb_thresh[1] = cpu_to_be16(0x151b);
> - ram_table->crgb_thresh[2] = cpu_to_be16(0x17d5);
> - ram_table->crgb_thresh[3] = cpu_to_be16(0x1a56);
> - ram_table->crgb_thresh[4] = cpu_to_be16(0x1c83);
> - ram_table->crgb_thresh[5] = cpu_to_be16(0x1e72);
> - ram_table->crgb_thresh[6] = cpu_to_be16(0x20f0);
> - ram_table->crgb_thresh[7] = cpu_to_be16(0x232b);
> - ram_table->crgb_offset[0] = cpu_to_be16(0x2999);
> - ram_table->crgb_offset[1] = cpu_to_be16(0x3999);
> - ram_table->crgb_offset[2] = cpu_to_be16(0x4666);
> - ram_table->crgb_offset[3] = cpu_to_be16(0x5999);
> - ram_table->crgb_offset[4] = cpu_to_be16(0x6333);
> - ram_table->crgb_offset[5] = cpu_to_be16(0x7800);
> - ram_table->crgb_offset[6] = cpu_to_be16(0x8c00);
> - ram_table->crgb_offset[7] = cpu_to_be16(0xa000);
> - ram_table->crgb_slope[0] = cpu_to_be16(0x3609);
> - ram_table->crgb_slope[1] = cpu_to_be16(0x2dfa);
> - ram_table->crgb_slope[2] = cpu_to_be16(0x27ea);
> - ram_table->crgb_slope[3] = cpu_to_be16(0x235d);
> - ram_table->crgb_slope[4] = cpu_to_be16(0x2042);
> - ram_table->crgb_slope[5] = cpu_to_be16(0x1dc3);
> - ram_table->crgb_slope[6] = cpu_to_be16(0x1b1a);
> - ram_table->crgb_slope[7] = cpu_to_be16(0x1910);
> + ram_table->crgb_thresh[0] = (big_endian) ? cpu_to_be16(0x127c)
> : 0x127c;
> + ram_table->crgb_thresh[1] = (big_endian) ? cpu_to_be16(0x151b)
> : 0x151b;
> + ram_table->crgb_thresh[2] = (big_endian) ? cpu_to_be16(0x17d5)
> : 0x17d5;
> + ram_table->crgb_thresh[3] = (big_endian) ? cpu_to_be16(0x1a56)
> : 0x1a56;
> + ram_table->crgb_thresh[4] = (big_endian) ? cpu_to_be16(0x1c83)
> : 0x1c83;
> + ram_table->crgb_thresh[5] = (big_endian) ? cpu_to_be16(0x1e72)
> : 0x1e72;
> + ram_table->crgb_thresh[6] = (big_endian) ? cpu_to_be16(0x20f0)
> : 0x20f0;
> + ram_table->crgb_thresh[7] = (big_endian) ? cpu_to_be16(0x232b)
> : 0x232b;
> + ram_table->crgb_offset[0] = (big_endian) ? cpu_to_be16(0x2999)
> : 0x2999;
> + ram_table->crgb_offset[1] = (big_endian) ? cpu_to_be16(0x3999)
> : 0x3999;
> + ram_table->crgb_offset[2] = (big_endian) ? cpu_to_be16(0x4666)
> : 0x4666;
> + ram_table->crgb_offset[3] = (big_endian) ? cpu_to_be16(0x5999)
> : 0x5999;
> + ram_table->crgb_offset[4] = (big_endian) ? cpu_to_be16(0x6333)
> : 0x6333;
> + ram_table->crgb_offset[5] = (big_endian) ? cpu_to_be16(0x7800)
> : 0x7800;
> + ram_table->crgb_offset[6] = (big_endian) ? cpu_to_be16(0x8c00)
> : 0x8c00;
> + ram_table->crgb_offset[7] = (big_endian) ? cpu_to_be16(0xa000)
> : 0xa000;
> + ram_table->crgb_slope[0] = (big_endian) ? cpu_to_be16(0x3609)
> : 0x3609;
> + ram_table->crgb_slope[1] = (big_endian) ? cpu_to_be16(0x2dfa)
> : 0x2dfa;
> + ram_table->crgb_slope[2] = (big_endian) ? cpu_to_be16(0x27ea)
> : 0x27ea;
> + ram_table->crgb_slope[3] = (big_endian) ? cpu_to_be16(0x235d)
> : 0x235d;
> + ram_table->crgb_slope[4] = (big_endian) ? cpu_to_be16(0x2042)
> : 0x2042;
> + ram_table->crgb_slope[5] = (big_endian) ? cpu_to_be16(0x1dc3)
> : 0x1dc3;
> + ram_table->crgb_slope[6] = (big_endian) ? cpu_to_be16(0x1b1a)
> : 0x1b1a;
> + ram_table->crgb_slope[7] = (big_endian) ? cpu_to_be16(0x1910)
> : 0x1910;
>
> fill_backlight_transform_table_v_2_2(
> - params, ram_table);
> + params, ram_table, big_endian);
> }
>
> bool dmub_init_abm_config(struct abm *abm,
> @@ -662,7 +660,7 @@ bool dmub_init_abm_config(struct abm *abm,
>
> memset(&ram_table, 0, sizeof(ram_table));
>
> - fill_iram_v_2_3((struct iram_table_v_2_2 *)ram_table, params);
> + fill_iram_v_2_3((struct iram_table_v_2_2 *)ram_table, params,
> false);
> result = abm->funcs->init_abm_config(
> abm, (char *)(&ram_table), IRAM_RESERVE_AREA_START_V2_2);
>
> @@ -684,11 +682,11 @@ bool dmcu_load_iram(struct dmcu *dmcu,
> memset(&ram_table, 0, sizeof(ram_table));
>
> if (dmcu->dmcu_version.abm_version == 0x24) {
> - fill_iram_v_2_3((struct iram_table_v_2_2 *)ram_table,
> params);
> + fill_iram_v_2_3((struct iram_table_v_2_2 *)ram_table,
> params, true);
> result = dmcu->funcs->load_iram(
> dmcu, 0, (char
> *)(&ram_table), IRAM_RESERVE_AREA_START_V2_2);
> } else if (dmcu->dmcu_version.abm_version == 0x23) {
> - fill_iram_v_2_3((struct iram_table_v_2_2 *)ram_table,
> params);
> + fill_iram_v_2_3((struct iram_table_v_2_2 *)ram_table,
> params, true);
>
> result = dmcu->funcs->load_iram(
> dmcu, 0, (char *)(&ram_table),
> IRAM_RESERVE_AREA_START_V2_2);
> --
> 2.26.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Calexander.deucher%40amd.com%7C71bc54a1a7b7444439c208d7e25fab51%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637226772877797082&sdata=sa4MJoUY%2FjVgW3f4Qx1N4KYpFY3QyqZPWVDbRoUmTxs%3D&reserved=0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20200417/7e62311b/attachment-0001.htm>
More information about the amd-gfx
mailing list