Re: [PATCH 3/6 v6] fbdev/simplefb: Add the N64's mode

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 1/13/21 4:11 PM, Lauri Kasanen wrote:
> By keeping it separate for the platform, we save 748 bytes.
> This brings simplefb to 2.5kb from 3.2kb, when a native fb driver
> was 1.5kb.
> 
> Signed-off-by: Lauri Kasanen <cand@gmx.com>
> ---
>  include/linux/platform_data/simplefb.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/linux/platform_data/simplefb.h b/include/linux/platform_data/simplefb.h
> index ca83376..c9d4f2c 100644
> --- a/include/linux/platform_data/simplefb.h
> +++ b/include/linux/platform_data/simplefb.h
> @@ -13,6 +13,12 @@
>  #include <linux/types.h>
> 
>  /* format array, use it to initialize a "struct simplefb_format" array */
> +#ifdef CONFIG_MACH_NINTENDO64
> +#define SIMPLEFB_FORMATS \
> +{ \
> +	{ "r5g5b5a1", 16, {11, 5}, {6, 5}, {1, 5}, {0, 1}, DRM_FORMAT_RGBA5551 }, \
> +}
> +#else
>  #define SIMPLEFB_FORMATS \
>  { \
>  	{ "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }, \
> @@ -25,6 +31,7 @@
>  	{ "x2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {0, 0}, DRM_FORMAT_XRGB2101010 }, \
>  	{ "a2r10g10b10", 32, {20, 10}, {10, 10}, {0, 10}, {30, 2}, DRM_FORMAT_ARGB2101010 }, \
>  }
> +#endif
> 
>  /*
>   * Data-Format for Simple-Framebuffers

I am honestly not entirely convinced of the #ifdef-ery this adds. I understand that RAM is
tight on the N64, but this saves just 700 bytes, which even on the N64 is not a lot.

Right now it might be just 1 #ifdef for the N64, but if other platforms follow this example?

Honestly I would prefer to just see the format added to the existing SIMPLEFB_FORMATS
define without the #ifdef.

Alternatively:

Are you using devicetree or are you using a board.c file to instantiate everything?
If it is the latter I guess we could add a const struct simplefb_format * to the pdata
and then just pass in the format directly. I can even imagine the code which uses
the SIMPLEFB_FORMATS having a #ifdef CONFIG_OF around it in that case to optimize it out...

That would be a more generic solution which hopefully will save as much if not more
space...

Regards,

Hans