Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
On 04/19, Christian König wrote: > Am 17.04.20 um 23:43 schrieb Rodrigo Siqueira: > > Hi, > > > > Wyatt made the below patch for fixing this issue. I can apply it on top > > of this patchset if you all agree. > > > > [Why] > > Current code does not guarantee the correct endianness of memory being > > copied to fw, specifically in the case where cpu isn't little endian. > > > > [How] > > Windows and Diags are always little endian, so we define a macro that > > does nothing. Linux already defines this macro and will do the correct > > endianness conversion. > > > > Signed-off-by: Wyatt Wood > > Reviewed-by: Harry Wentland > > Acked-by: Anthony Koo > > Acked-by: Rodrigo Siqueira > > --- > > .../amd/display/modules/power/power_helpers.c | 58 ++- > > 1 file changed, 31 insertions(+), 27 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 edb446455f6b..8c37bcc27132 100644 > > --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c > > +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c > > @@ -265,9 +265,11 @@ static void > > fill_backlight_transform_table_v_2_2(struct dmcu_iram_parameters par > > ASSERT(lut_index < params.backlight_lut_array_size); > > table->backlight_thresholds[i] = (big_endian) ? > > - cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) : > > DIV_ROUNDUP((i * 65536), num_entries); > > + cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) : > > + cpu_to_le16(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]; > > + cpu_to_be16(params.backlight_lut_array[lut_index]) : > > + cpu_to_le16(params.backlight_lut_array[lut_index]); > > } > > } > > @@ -596,7 +598,9 @@ void fill_iram_v_2_3(struct iram_table_v_2_2 > > *ram_table, struct dmcu_iram_parame > > unsigned int set = params.set; > > ram_table->flags = 0x0; > > - ram_table->min_abm_backlight = (big_endian) ? > > cpu_to_be16(params.min_abm_backlight) : params.min_abm_backlight; > > + ram_table->min_abm_backlight = (big_endian) ? > > + cpu_to_be16(params.min_abm_backlight) : > > + cpu_to_le16(params.min_abm_backlight); > > for (i = 0; i < NUM_AGGR_LEVEL; i++) { > > ram_table->hybrid_factor[i] = > > abm_settings[set][i].brightness_gain; > > @@ -620,30 +624,30 @@ 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] = (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; > > + ram_table->crgb_thresh[0] = (big_endian) ? cpu_to_be16(0x127c) : > > cpu_to_le16(0x127c); > > + ram_table->crgb_thresh[1] = (big_endian) ? cpu_to_be16(0x151b) : > > cpu_to_le16(0x151b); > > + ram_table->crgb_thresh[2] = (big_endian
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
Am 17.04.20 um 23:43 schrieb Rodrigo Siqueira: Hi, Wyatt made the below patch for fixing this issue. I can apply it on top of this patchset if you all agree. [Why] Current code does not guarantee the correct endianness of memory being copied to fw, specifically in the case where cpu isn't little endian. [How] Windows and Diags are always little endian, so we define a macro that does nothing. Linux already defines this macro and will do the correct endianness conversion. Signed-off-by: Wyatt Wood Reviewed-by: Harry Wentland Acked-by: Anthony Koo Acked-by: Rodrigo Siqueira --- .../amd/display/modules/power/power_helpers.c | 58 ++- 1 file changed, 31 insertions(+), 27 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 edb446455f6b..8c37bcc27132 100644 --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c @@ -265,9 +265,11 @@ static void fill_backlight_transform_table_v_2_2(struct dmcu_iram_parameters par ASSERT(lut_index < params.backlight_lut_array_size); table->backlight_thresholds[i] = (big_endian) ? - cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) : DIV_ROUNDUP((i * 65536), num_entries); + cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) : + cpu_to_le16(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]; + cpu_to_be16(params.backlight_lut_array[lut_index]) : + cpu_to_le16(params.backlight_lut_array[lut_index]); } } @@ -596,7 +598,9 @@ void fill_iram_v_2_3(struct iram_table_v_2_2 *ram_table, struct dmcu_iram_parame unsigned int set = params.set; ram_table->flags = 0x0; - ram_table->min_abm_backlight = (big_endian) ? cpu_to_be16(params.min_abm_backlight) : params.min_abm_backlight; + ram_table->min_abm_backlight = (big_endian) ? + cpu_to_be16(params.min_abm_backlight) : + cpu_to_le16(params.min_abm_backlight); for (i = 0; i < NUM_AGGR_LEVEL; i++) { ram_table->hybrid_factor[i] = abm_settings[set][i].brightness_gain; @@ -620,30 +624,30 @@ 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] = (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; + ram_table->crgb_thresh[0] = (big_endian) ? cpu_to_be16(0x127c) : cpu_to_le16(0x127c); + ram_table->crgb_thresh[1] = (big_endian) ? cpu_to_be16(0x151b) : cpu_to_le16(0x151b); + ram_table->crgb_thresh[2] = (big_endian) ? cpu_to_be16(0x17d5) : cpu_to_le16(0x17d5); + ram_table->crgb_thresh[3] = (big_endian) ? cpu_to_be16(0x1a56) : cpu_to_le16(0x1a56); + ram_table->crgb_thresh[4] = (big_endian) ? cpu_to_
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
Am 17.04.20 um 17:40 schrieb Harry Wentland: On 2020-04-17 8:09 a.m., Christian König wrote: Am 17.04.20 um 12:43 schrieb Michel Dänzer: On 2020-04-17 11:22 a.m., Christian König wrote: 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. The *b*e macros aren't NOPs on little endian architectures like x86, they are on big endian architectures. Vice versa for the *l*e macros. Yeah, that's what I meant with "if everything is done right" :) I usually can't remember what does what with those functions. Christian. I think key here is that dmcub FW is little endian, whereas the old dmcu FW was big endian. Hence we had the cpu_to_be conversion here for the old dmcu. Now it looks like we want to reuse the same function for dmcub calls and hence need to ensure we're not converting values to big-endian. The big_endian parameter is specifying the endianness of the FW. The right approach would be to do cpu_to_be for dmcu and cpu_to_le for dmcub. Thanks for the explanation, that makes it much more clear what should happen here. Christian. Harry ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
Hi, Wyatt made the below patch for fixing this issue. I can apply it on top of this patchset if you all agree. [Why] Current code does not guarantee the correct endianness of memory being copied to fw, specifically in the case where cpu isn't little endian. [How] Windows and Diags are always little endian, so we define a macro that does nothing. Linux already defines this macro and will do the correct endianness conversion. Signed-off-by: Wyatt Wood Reviewed-by: Harry Wentland Acked-by: Anthony Koo Acked-by: Rodrigo Siqueira --- .../amd/display/modules/power/power_helpers.c | 58 ++- 1 file changed, 31 insertions(+), 27 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 edb446455f6b..8c37bcc27132 100644 --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c @@ -265,9 +265,11 @@ static void fill_backlight_transform_table_v_2_2(struct dmcu_iram_parameters par ASSERT(lut_index < params.backlight_lut_array_size); table->backlight_thresholds[i] = (big_endian) ? - cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) : DIV_ROUNDUP((i * 65536), num_entries); + cpu_to_be16(DIV_ROUNDUP((i * 65536), num_entries)) : + cpu_to_le16(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]; + cpu_to_be16(params.backlight_lut_array[lut_index]) : + cpu_to_le16(params.backlight_lut_array[lut_index]); } } @@ -596,7 +598,9 @@ void fill_iram_v_2_3(struct iram_table_v_2_2 *ram_table, struct dmcu_iram_parame unsigned int set = params.set; ram_table->flags = 0x0; - ram_table->min_abm_backlight = (big_endian) ? cpu_to_be16(params.min_abm_backlight) : params.min_abm_backlight; + ram_table->min_abm_backlight = (big_endian) ? + cpu_to_be16(params.min_abm_backlight) : + cpu_to_le16(params.min_abm_backlight); for (i = 0; i < NUM_AGGR_LEVEL; i++) { ram_table->hybrid_factor[i] = abm_settings[set][i].brightness_gain; @@ -620,30 +624,30 @@ 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] = (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; + ram_table->crgb_thresh[0] = (big_endian) ? cpu_to_be16(0x127c) : cpu_to_le16(0x127c); + ram_table->crgb_thresh[1] = (big_endian) ? cpu_to_be16(0x151b) : cpu_to_le16(0x151b); + ram_table->crgb_thresh[2] = (big_endian) ? cpu_to_be16(0x17d5) : cpu_to_le16(0x17d5); + ram_table->crgb_thresh[3] = (big_endian) ? cpu_to_be16(0x1a56) : cpu_to_le16(0x1a56); + ram_table->crgb_thresh[4] = (big_endian) ? cpu_to_be16(0x1c83) : cpu_to_le16(0x1c83
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
On 2020-04-17 8:09 a.m., Christian König wrote: > Am 17.04.20 um 12:43 schrieb Michel Dänzer: >> On 2020-04-17 11:22 a.m., Christian König wrote: >>> 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. >> The *b*e macros aren't NOPs on little endian architectures like x86, >> they are on big endian architectures. Vice versa for the *l*e macros. > > Yeah, that's what I meant with "if everything is done right" :) > > I usually can't remember what does what with those functions. > > Christian. I think key here is that dmcub FW is little endian, whereas the old dmcu FW was big endian. Hence we had the cpu_to_be conversion here for the old dmcu. Now it looks like we want to reuse the same function for dmcub calls and hence need to ensure we're not converting values to big-endian. The big_endian parameter is specifying the endianness of the FW. The right approach would be to do cpu_to_be for dmcu and cpu_to_le for dmcub. Harry ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
Am 17.04.20 um 12:43 schrieb Michel Dänzer: On 2020-04-17 11:22 a.m., Christian König wrote: 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. The *b*e macros aren't NOPs on little endian architectures like x86, they are on big endian architectures. Vice versa for the *l*e macros. Yeah, that's what I meant with "if everything is done right" :) I usually can't remember what does what with those functions. Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
On 2020-04-17 11:22 a.m., Christian König wrote: > 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. The *b*e macros aren't NOPs on little endian architectures like x86, they are on big endian architectures. Vice versa for the *l*e macros. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
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 on behalf of Rodrigo Siqueira *Sent:* Thursday, April 16, 2020 7:40 PM *To:* amd-gfx@lists.freedesktop.org *Cc:* Li, Sun peng (Leo) ; Wentland, Harry ; Siqueira, Rodrigo ; Wood, Wyatt ; Lakha, Bhawanpreet ; Koo, Anthony *Subject:* [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table From: Wyatt Wood [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 Reviewed-by: Anthony Koo Acked-by: Rodrigo Siqueira --- .../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
Re: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
[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 on behalf of Rodrigo Siqueira Sent: Thursday, April 16, 2020 7:40 PM To: amd-gfx@lists.freedesktop.org Cc: Li, Sun peng (Leo) ; Wentland, Harry ; Siqueira, Rodrigo ; Wood, Wyatt ; Lakha, Bhawanpreet ; Koo, Anthony Subject: [PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table From: Wyatt Wood [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 Reviewed-by: Anthony Koo Acked-by: Rodrigo Siqueira --- .../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_
[PATCH 05/35] drm/amd/display: Remove byte swapping for dmcub abm config table
From: Wyatt Wood [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 Reviewed-by: Anthony Koo Acked-by: Rodrigo Siqueira --- .../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(0x1