Re: [PATCH] drm/fourcc: fix Amlogic format modifier masks

2021-01-11 Thread Neil Armstrong
Hi,

On 10/01/2021 13:51, Simon Ser wrote:
> The comment says the layout and options use 8 bits, and the shift
> uses 8 bits. However the mask is 0xf, ie. 0b (4 bits).
> 
> This could be surprising when introducing new layouts or options
> that take more than 4 bits, as this would silently drop the high
> bits.

Indeed, but the masks are "private", and would be updated accordingly when 
introducing new layouts.

> 
> Make the masks consistent with the comment and the shift.
> 
> Found when writing a drm_info patch [1].
> 
> [1]: https://github.com/ascent12/drm_info/pull/67
> 
> Signed-off-by: Simon Ser 
> Fixes: d6528ec88309 ("drm/fourcc: Add modifier definitions for describing 
> Amlogic Video Framebuffer Compression")
> Cc: Neil Armstrong 
> Cc: Sam Ravnborg 
> Cc: Kevin Hilman 
> Cc: Daniel Vetter 
> ---
>  include/uapi/drm/drm_fourcc.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 723c8e23ca87..5f42a14481bd 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -1036,9 +1036,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 
> modifier)
>   * Not all combinations are valid, and different SoCs may support different
>   * combinations of layout and options.
>   */
> -#define __fourcc_mod_amlogic_layout_mask 0xf
> +#define __fourcc_mod_amlogic_layout_mask 0xff
>  #define __fourcc_mod_amlogic_options_shift 8
> -#define __fourcc_mod_amlogic_options_mask 0xf
> +#define __fourcc_mod_amlogic_options_mask 0xff
>  
>  #define DRM_FORMAT_MOD_AMLOGIC_FBC(__layout, __options) \
>   fourcc_mod_code(AMLOGIC, \
> 

Anyway:
Acked-by: Neil Armstrong 

Thanks,
Neil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/fourcc: fix Amlogic format modifier masks

2021-01-10 Thread Simon Ser
The comment says the layout and options use 8 bits, and the shift
uses 8 bits. However the mask is 0xf, ie. 0b (4 bits).

This could be surprising when introducing new layouts or options
that take more than 4 bits, as this would silently drop the high
bits.

Make the masks consistent with the comment and the shift.

Found when writing a drm_info patch [1].

[1]: https://github.com/ascent12/drm_info/pull/67

Signed-off-by: Simon Ser 
Fixes: d6528ec88309 ("drm/fourcc: Add modifier definitions for describing 
Amlogic Video Framebuffer Compression")
Cc: Neil Armstrong 
Cc: Sam Ravnborg 
Cc: Kevin Hilman 
Cc: Daniel Vetter 
---
 include/uapi/drm/drm_fourcc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 723c8e23ca87..5f42a14481bd 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -1036,9 +1036,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
  * Not all combinations are valid, and different SoCs may support different
  * combinations of layout and options.
  */
-#define __fourcc_mod_amlogic_layout_mask 0xf
+#define __fourcc_mod_amlogic_layout_mask 0xff
 #define __fourcc_mod_amlogic_options_shift 8
-#define __fourcc_mod_amlogic_options_mask 0xf
+#define __fourcc_mod_amlogic_options_mask 0xff
 
 #define DRM_FORMAT_MOD_AMLOGIC_FBC(__layout, __options) \
fourcc_mod_code(AMLOGIC, \
-- 
2.30.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel