2017-08-08 08:10:35

by Michal Simek

[permalink] [raw]
Subject: [PATCH 1/2] video: fbdev: Fix multiple style issues in xilinxfb

From: Hyun Kwon <[email protected]>

All reported by from checkpatch
./scripts/checkpatch.pl --max-line-length 120 -strict -f
drivers/video/fbdev/xilinxfb.c

WARNING: Block comments should align the * on each line
WARNING: Block comments use a trailing */ on a separate line
WARNING: Block comments use * on subsequent lines
WARNING: please, no space before tabs
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
WARNING: braces {} are not necessary for single statement blocks
WARNING: Missing a blank line after declarations
WARNING: struct of_device_id should normally be const
CHECK: Please don't use multiple blank lines
CHECK: Blank lines aren't necessary after an open brace '{'
CHECK: Alignment should match open parenthesis
CHECK: 'Endianess' may be misspelled - perhaps 'Endianness'?
CHECK: spaces preferred around that '*' (ctx:VxV)
ERROR: that open brace { should be on the previous line

Signed-off-by: Hyun Kwon <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---

drivers/video/fbdev/xilinxfb.c | 56 +++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/video/fbdev/xilinxfb.c b/drivers/video/fbdev/xilinxfb.c
index 17dc119c7a98..0d828e46d704 100644
--- a/drivers/video/fbdev/xilinxfb.c
+++ b/drivers/video/fbdev/xilinxfb.c
@@ -41,7 +41,6 @@

#define DRIVER_NAME "xilinxfb"

-
/*
* Xilinx calls it "TFT LCD Controller" though it can also be used for
* the VGA port on the Xilinx ML40x board. This is a hardware display
@@ -92,8 +91,9 @@ struct xilinxfb_platform_data {
u32 xvirt, yvirt; /* resolution of memory buffer */

/* Physical address of framebuffer memory; If non-zero, driver
- * will use provided memory address instead of allocating one from
- * the consistent pool. */
+ * will use provided memory address instead of allocating one from
+ * the consistent pool.
+ */
u32 fb_phys;
};

@@ -128,18 +128,18 @@ struct xilinxfb_platform_data {
.activate = FB_ACTIVATE_NOW
};

-
#define BUS_ACCESS_FLAG 0x1 /* 1 = BUS, 0 = DCR */
#define LITTLE_ENDIAN_ACCESS 0x2 /* LITTLE ENDIAN IO functions */

struct xilinxfb_drvdata {
-
struct fb_info info; /* FB driver info record */

phys_addr_t regs_phys; /* phys. address of the control
- registers */
+ * registers
+ */
void __iomem *regs; /* virt. address of the control
- registers */
+ * registers
+ */
#ifdef CONFIG_PPC_DCR
dcr_host_t dcr_host;
unsigned int dcr_len;
@@ -148,7 +148,7 @@ struct xilinxfb_drvdata {
dma_addr_t fb_phys; /* phys. address of the frame buffer */
int fb_alloced; /* Flag, was the fb memory alloced? */

- u8 flags; /* features of the driver */
+ u8 flags; /* features of the driver */

u32 reg_ctrl_default;

@@ -165,7 +165,7 @@ struct xilinxfb_drvdata {
* which bus its connected and call the appropriate write API.
*/
static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
- u32 val)
+ u32 val)
{
if (drvdata->flags & BUS_ACCESS_FLAG) {
if (drvdata->flags & LITTLE_ENDIAN_ACCESS)
@@ -195,8 +195,8 @@ static u32 xilinx_fb_in32(struct xilinxfb_drvdata *drvdata, u32 offset)
}

static int
-xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
- unsigned transp, struct fb_info *fbi)
+xilinx_fb_setcolreg(unsigned int regno, unsigned int red, unsigned int green,
+ unsigned int blue, unsigned int transp, struct fb_info *fbi)
{
u32 *palette = fbi->pseudo_palette;

@@ -205,9 +205,11 @@ static u32 xilinx_fb_in32(struct xilinxfb_drvdata *drvdata, u32 offset)

if (fbi->var.grayscale) {
/* Convert color to grayscale.
- * grayscale = 0.30*R + 0.59*G + 0.11*B */
- red = green = blue =
- (red * 77 + green * 151 + blue * 28 + 127) >> 8;
+ * grayscale = 0.30*R + 0.59*G + 0.11*B
+ */
+ blue = (red * 77 + green * 151 + blue * 28 + 127) >> 8;
+ green = blue;
+ red = green;
}

/* fbi->fix.visual is always FB_VISUAL_TRUECOLOR */
@@ -241,13 +243,11 @@ static u32 xilinx_fb_in32(struct xilinxfb_drvdata *drvdata, u32 offset)
xilinx_fb_out32(drvdata, REG_CTRL, 0);
default:
break;
-
}
return 0; /* success */
}

-static struct fb_ops xilinxfb_ops =
-{
+static struct fb_ops xilinxfb_ops = {
.owner = THIS_MODULE,
.fb_setcolreg = xilinx_fb_setcolreg,
.fb_blank = xilinx_fb_blank,
@@ -286,7 +286,8 @@ static int xilinxfb_assign(struct platform_device *pdev,
} else {
drvdata->fb_alloced = 1;
drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
- &drvdata->fb_phys, GFP_KERNEL);
+ &drvdata->fb_phys,
+ GFP_KERNEL);
}

if (!drvdata->fb_virt) {
@@ -300,7 +301,7 @@ static int xilinxfb_assign(struct platform_device *pdev,
/* Tell the hardware where the frame buffer is */
xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
rc = xilinx_fb_in32(drvdata, REG_FB_ADDR);
- /* Endianess detection */
+ /* Endianness detection */
if (rc != drvdata->fb_phys) {
drvdata->flags |= LITTLE_ENDIAN_ACCESS;
xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
@@ -310,8 +311,7 @@ static int xilinxfb_assign(struct platform_device *pdev,
drvdata->reg_ctrl_default = REG_CTRL_ENABLE;
if (pdata->rotate_screen)
drvdata->reg_ctrl_default |= REG_CTRL_ROTATE;
- xilinx_fb_out32(drvdata, REG_CTRL,
- drvdata->reg_ctrl_default);
+ xilinx_fb_out32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);

/* Fill struct fb_info */
drvdata->info.device = dev;
@@ -364,7 +364,7 @@ static int xilinxfb_assign(struct platform_device *pdev,
err_cmap:
if (drvdata->fb_alloced)
dma_free_coherent(dev, PAGE_ALIGN(fbsize), drvdata->fb_virt,
- drvdata->fb_phys);
+ drvdata->fb_phys);
else
iounmap(drvdata->fb_virt);

@@ -435,12 +435,12 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
* Fill the resource structure if its direct BUS interface
* otherwise fill the dcr_host structure.
*/
- if (tft_access) {
+ if (tft_access)
drvdata->flags |= BUS_ACCESS_FLAG;
- }
#ifdef CONFIG_PPC_DCR
else {
int start;
+
start = dcr_resource_start(pdev->dev.of_node, 0);
drvdata->dcr_len = dcr_resource_len(pdev->dev.of_node, 0);
drvdata->dcr_host = dcr_map(pdev->dev.of_node, start, drvdata->dcr_len);
@@ -452,19 +452,19 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
#endif

prop = of_get_property(pdev->dev.of_node, "phys-size", &size);
- if ((prop) && (size >= sizeof(u32)*2)) {
+ if ((prop) && (size >= sizeof(u32) * 2)) {
pdata.screen_width_mm = prop[0];
pdata.screen_height_mm = prop[1];
}

prop = of_get_property(pdev->dev.of_node, "resolution", &size);
- if ((prop) && (size >= sizeof(u32)*2)) {
+ if ((prop) && (size >= sizeof(u32) * 2)) {
pdata.xres = prop[0];
pdata.yres = prop[1];
}

prop = of_get_property(pdev->dev.of_node, "virtual-resolution", &size);
- if ((prop) && (size >= sizeof(u32)*2)) {
+ if ((prop) && (size >= sizeof(u32) * 2)) {
pdata.xvirt = prop[0];
pdata.yvirt = prop[1];
}
@@ -482,7 +482,7 @@ static int xilinxfb_of_remove(struct platform_device *op)
}

/* Match table for of_platform binding */
-static struct of_device_id xilinxfb_of_match[] = {
+static const struct of_device_id xilinxfb_of_match[] = {
{ .compatible = "xlnx,xps-tft-1.00.a", },
{ .compatible = "xlnx,xps-tft-2.00.a", },
{ .compatible = "xlnx,xps-tft-2.01.a", },
--
1.9.1


2017-08-08 08:10:37

by Michal Simek

[permalink] [raw]
Subject: [PATCH 2/2] video: fbdev: Enable Xilinx FB for ZynqMP

Enable this driver for Xilinx ZynqMP.

Signed-off-by: Michal Simek <[email protected]>
---

drivers/video/fbdev/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 5c6696bb56da..5e58f5ec0a28 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2173,7 +2173,7 @@ config FB_PS3_DEFAULT_SIZE_M

config FB_XILINX
tristate "Xilinx frame buffer support"
- depends on FB && (XILINX_VIRTEX || MICROBLAZE || ARCH_ZYNQ)
+ depends on FB && (XILINX_VIRTEX || MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
1.9.1

2017-08-14 13:42:28

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 2/2] video: fbdev: Enable Xilinx FB for ZynqMP

On Tue, Aug 8, 2017 at 10:10 AM, Michal Simek <[email protected]> wrote:

> Enable this driver for Xilinx ZynqMP.
>
> Signed-off-by: Michal Simek <[email protected]>

Not a comment on the patch per se, but I would advice you to look into
migrating the Xilinx frame buffer to DRI/DRM/KMS.

The recent CMA helpers has made it very simple and focused to write
DRM drivers. See for example drivers/gpu/drm/pl111

Yours,
Linus Walleij

2017-08-14 13:46:19

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH 2/2] video: fbdev: Enable Xilinx FB for ZynqMP

On 14.8.2017 15:42, Linus Walleij wrote:
> On Tue, Aug 8, 2017 at 10:10 AM, Michal Simek <[email protected]> wrote:
>
>> Enable this driver for Xilinx ZynqMP.
>>
>> Signed-off-by: Michal Simek <[email protected]>
>
> Not a comment on the patch per se, but I would advice you to look into
> migrating the Xilinx frame buffer to DRI/DRM/KMS.
>
> The recent CMA helpers has made it very simple and focused to write
> DRM drivers. See for example drivers/gpu/drm/pl111

There is a lot of work done by xilinx in this area gpu/drm/kms. I am
trying to upstream what xilinx has in soc tree and this change is there.
Maybe in this case it is useless and none is using it but it is there
and I don't want to break user. There shouldn't be any technical problem
to run this old fb driver in pl on arm64.

Thanks,
Michal



2017-08-14 13:49:05

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 2/2] video: fbdev: Enable Xilinx FB for ZynqMP

On Mon, Aug 14, 2017 at 3:46 PM, Michal Simek <[email protected]> wrote:
> On 14.8.2017 15:42, Linus Walleij wrote:
>> On Tue, Aug 8, 2017 at 10:10 AM, Michal Simek <[email protected]> wrote:
>>
>>> Enable this driver for Xilinx ZynqMP.
>>>
>>> Signed-off-by: Michal Simek <[email protected]>
>>
>> Not a comment on the patch per se, but I would advice you to look into
>> migrating the Xilinx frame buffer to DRI/DRM/KMS.
>>
>> The recent CMA helpers has made it very simple and focused to write
>> DRM drivers. See for example drivers/gpu/drm/pl111
>
> There is a lot of work done by xilinx in this area gpu/drm/kms. I am
> trying to upstream what xilinx has in soc tree and this change is there.
> Maybe in this case it is useless and none is using it but it is there
> and I don't want to break user. There shouldn't be any technical problem
> to run this old fb driver in pl on arm64.

A proper drm kms driver gives you backwards compat for fbdev for free.
And we're taking in patches to fill feature gaps in our fbdev
emulation as people hit them. There no reason nowadays at all to merge
new fbdev native drivers, your users will be taken care of by a drm
kms driver fully.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2017-08-14 13:51:09

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH 2/2] video: fbdev: Enable Xilinx FB for ZynqMP

On 14.8.2017 15:49, Daniel Vetter wrote:
> On Mon, Aug 14, 2017 at 3:46 PM, Michal Simek <[email protected]> wrote:
>> On 14.8.2017 15:42, Linus Walleij wrote:
>>> On Tue, Aug 8, 2017 at 10:10 AM, Michal Simek <[email protected]> wrote:
>>>
>>>> Enable this driver for Xilinx ZynqMP.
>>>>
>>>> Signed-off-by: Michal Simek <[email protected]>
>>>
>>> Not a comment on the patch per se, but I would advice you to look into
>>> migrating the Xilinx frame buffer to DRI/DRM/KMS.
>>>
>>> The recent CMA helpers has made it very simple and focused to write
>>> DRM drivers. See for example drivers/gpu/drm/pl111
>>
>> There is a lot of work done by xilinx in this area gpu/drm/kms. I am
>> trying to upstream what xilinx has in soc tree and this change is there.
>> Maybe in this case it is useless and none is using it but it is there
>> and I don't want to break user. There shouldn't be any technical problem
>> to run this old fb driver in pl on arm64.
>
> A proper drm kms driver gives you backwards compat for fbdev for free.
> And we're taking in patches to fill feature gaps in our fbdev
> emulation as people hit them. There no reason nowadays at all to merge
> new fbdev native drivers, your users will be taken care of by a drm
> kms driver fully.

You can look at xilinx work in this area.
https://github.com/Xilinx/linux-xlnx
Unfortunately they haven't started to upstream it.

In this particular case I am not aware about anybody who is going to
write new driver for this soft IP.

Thanks,
Michal


Subject: Re: [PATCH 1/2] video: fbdev: Fix multiple style issues in xilinxfb

On Tuesday, August 08, 2017 10:10:25 AM Michal Simek wrote:
> From: Hyun Kwon <[email protected]>
>
> All reported by from checkpatch
> ./scripts/checkpatch.pl --max-line-length 120 -strict -f
> drivers/video/fbdev/xilinxfb.c
>
> WARNING: Block comments should align the * on each line
> WARNING: Block comments use a trailing */ on a separate line
> WARNING: Block comments use * on subsequent lines
> WARNING: please, no space before tabs
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> WARNING: braces {} are not necessary for single statement blocks
> WARNING: Missing a blank line after declarations
> WARNING: struct of_device_id should normally be const
> CHECK: Please don't use multiple blank lines
> CHECK: Blank lines aren't necessary after an open brace '{'
> CHECK: Alignment should match open parenthesis
> CHECK: 'Endianess' may be misspelled - perhaps 'Endianness'?
> CHECK: spaces preferred around that '*' (ctx:VxV)
> ERROR: that open brace { should be on the previous line
>
> Signed-off-by: Hyun Kwon <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>

Patch queued for 4.14, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Subject: Re: [PATCH 2/2] video: fbdev: Enable Xilinx FB for ZynqMP

On Tuesday, August 08, 2017 10:10:26 AM Michal Simek wrote:
> Enable this driver for Xilinx ZynqMP.
>
> Signed-off-by: Michal Simek <[email protected]>

Patch queued for 4.14, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics