From: Sam Ravnborg <sam@ravnborg.org>
To: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 3/6] video: fbdev: controlfb: add COMPILE_TEST support
Date: Fri, 17 Jan 2020 20:31:12 +0100 [thread overview]
Message-ID: <20200117193112.GC24812@ravnborg.org> (raw)
In-Reply-To: <20200116140900.26363-4-b.zolnierkie@samsung.com>
Hi Bartlomiej
On Thu, Jan 16, 2020 at 03:08:57PM +0100, Bartlomiej Zolnierkiewicz wrote:
> Add COMPILE_TEST support to controlfb driver for better compile
> testing coverage.
This is not a nice patch to add COMPILE_TEST support :-(
But I see why you do it.
I already spent too much time being side-tracked by this, but here are
some comments to consider.
With the comments considered:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
> drivers/video/fbdev/Kconfig | 2 +-
> drivers/video/fbdev/controlfb.c | 21 +++++++++++++++++++--
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index aa9541bf964b..91c872457863 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -472,7 +472,7 @@ config FB_OF
>
> config FB_CONTROL
> bool "Apple \"control\" display support"
> - depends on (FB = y) && PPC_PMAC && PPC32
> + depends on (FB = y) && ((PPC_PMAC && PPC32) || COMPILE_TEST)
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
> index bd0f61d8bdb5..87cd817ad4c6 100644
> --- a/drivers/video/fbdev/controlfb.c
> +++ b/drivers/video/fbdev/controlfb.c
> @@ -47,12 +47,25 @@
> #include <linux/nvram.h>
> #include <linux/adb.h>
> #include <linux/cuda.h>
> +#ifdef CONFIG_PPC_PMAC
> #include <asm/prom.h>
> #include <asm/btext.h>
> +#endif
>
> #include "macmodes.h"
> #include "controlfb.h"
>
> +#ifndef CONFIG_PPC_PMAC
> +#undef in_8
> +#undef out_8
> +#undef in_le32
> +#undef out_le32
> +#define in_8(addr) 0
> +#define out_8(addr, val)
> +#define in_le32(addr) 0
> +#define out_le32(addr, val)
> +#endif
> +
> struct fb_par_control {
> int vmode, cmode;
> int xres, yres;
> @@ -278,7 +291,9 @@ static int controlfb_mmap(struct fb_info *info,
> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> } else {
> /* framebuffer */
> +#ifdef CONFIG_PPC_PMAC
> vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
> +#endif
Add:
#define pgprot_cached_wthru(x) 0
in the CONFIG_PPC_PMAC block?
> }
>
> return vm_iomap_memory(vma, start, len);
> @@ -582,13 +597,14 @@ static void __init find_vram_size(struct fb_info_control *p)
>
> out_8(&p->frame_buffer[0x600000], 0xb3);
> out_8(&p->frame_buffer[0x600001], 0x71);
> +#ifdef CONFIG_PPC_PMAC
> asm volatile("eieio; dcbf 0,%0" : : "r" (&p->frame_buffer[0x600000])
> : "memory" );
> mb();
> asm volatile("eieio; dcbi 0,%0" : : "r" (&p->frame_buffer[0x600000])
> : "memory" );
> mb();
> -
> +#endif
The inline asm block could be written as:
static void invalid_vram_cache(void * addr)
{
eieio();
dcbf(addr);
mb;
eieio();
dcbi(addr);
mb();
}
And then this inline function could be in the CONFIG_PPC_PMAC block -
and a dummy in the else part.
The function name is just my best guess what the assembler does.
> bank2 = (in_8(&p->frame_buffer[0x600000]) == 0xb3)
> && (in_8(&p->frame_buffer[0x600001]) == 0x71);
>
> @@ -601,13 +617,14 @@ static void __init find_vram_size(struct fb_info_control *p)
>
> out_8(&p->frame_buffer[0], 0x5a);
> out_8(&p->frame_buffer[1], 0xc7);
> +#ifdef CONFIG_PPC_PMAC
> asm volatile("eieio; dcbf 0,%0" : : "r" (&p->frame_buffer[0])
> : "memory" );
> mb();
> asm volatile("eieio; dcbi 0,%0" : : "r" (&p->frame_buffer[0])
> : "memory" );
> mb();
> -
> +#endif
Same here.
> bank1 = (in_8(&p->frame_buffer[0]) == 0x5a)
> && (in_8(&p->frame_buffer[1]) == 0xc7);
>
> --
> 2.24.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-01-17 19:31 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20200116140914eucas1p1a62794ad40589e818614176ea8e817ff@eucas1p1.samsung.com>
2020-01-16 14:08 ` [PATCH 0/6] video: fbdev: controlfb: small cleanup Bartlomiej Zolnierkiewicz
[not found] ` <CGME20200116140915eucas1p2d6a2c654a66a78b6c3c1fc710f8a65b8@eucas1p2.samsung.com>
2020-01-16 14:08 ` [PATCH 1/6] video: fbdev: controlfb: fix sparse warning about using incorrect type Bartlomiej Zolnierkiewicz
2020-01-17 19:13 ` Sam Ravnborg
[not found] ` <CGME20200116140915eucas1p1d3696cbe492d55a4e4946c7e2d13f9d2@eucas1p1.samsung.com>
2020-01-16 14:08 ` [PATCH 2/6] video: fbdev: controlfb: remove obsolete module support Bartlomiej Zolnierkiewicz
[not found] ` <CGME20200116140915eucas1p28dfeecf8a58cecb00262fe86fd19c4f5@eucas1p2.samsung.com>
2020-01-16 14:08 ` [PATCH 3/6] video: fbdev: controlfb: add COMPILE_TEST support Bartlomiej Zolnierkiewicz
2020-01-17 19:31 ` Sam Ravnborg [this message]
[not found] ` <CGME20200116140916eucas1p1a6aff99bd371c4e23b1cc287bc3c42a8@eucas1p1.samsung.com>
2020-01-16 14:08 ` [PATCH 4/6] video: fbdev: controlfb: remove function prototypes part #1 Bartlomiej Zolnierkiewicz
[not found] ` <CGME20200116140916eucas1p112ff962e85e6feb4dbc2ff65443ef1ce@eucas1p1.samsung.com>
2020-01-16 14:08 ` [PATCH 5/6] video: fbdev: controlfb: remove function prototypes part #2 Bartlomiej Zolnierkiewicz
[not found] ` <CGME20200116140916eucas1p2bd3480995a55fdc646481a8fa5a5aff3@eucas1p2.samsung.com>
2020-01-16 14:09 ` [PATCH 6/6] video: fbdev: controlfb: remove function prototypes part #3 Bartlomiej Zolnierkiewicz
2020-01-17 19:32 ` [PATCH 0/6] video: fbdev: controlfb: small cleanup Sam Ravnborg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200117193112.GC24812@ravnborg.org \
--to=sam@ravnborg.org \
--cc=b.zolnierkie@samsung.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).