* [PATCH v3 1/2] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-29 14:40 UTC (permalink / raw)
To: andy
Cc: Abdun Nihaal, dan.carpenter, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <cover.1751207100.git.abdun.nihaal@gmail.com>
After commit 56c134f7f1b5 ("fbdev: Track deferred-I/O pages in pageref
struct"), fb_deferred_io_init() allocates memory for info->pagerefs as
well as return an error code on failure. However the error code is
ignored here and the memory allocated could leak because of not calling
fb_deferred_io_cleanup() on the error path.
Fix them by adding the cleanup function on the error path, and handling
the error code returned by fb_deferred_io_init().
Fixes: 56c134f7f1b5 ("fbdev: Track deferred-I/O pages in pageref struct")
Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
---
v2->v3: No change
v1->v2:
- Handle the error code returned by fb_deferred_io_init correctly
- Update Fixes tag to point to the commit that introduced the memory
allocation which leads to the leak.
drivers/staging/fbtft/fbtft-core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index da9c64152a60..8538b6bab6a5 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -612,7 +612,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
info->fix.line_length = width * bpp / 8;
info->fix.accel = FB_ACCEL_NONE;
info->fix.smem_len = vmem_size;
- fb_deferred_io_init(info);
+ if (fb_deferred_io_init(info))
+ goto release_framebuf;
info->var.rotate = pdata->rotate;
info->var.xres = width;
@@ -652,7 +653,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
if (par->gamma.curves && gamma) {
if (fbtft_gamma_parse_str(par, par->gamma.curves, gamma,
strlen(gamma)))
- goto release_framebuf;
+ goto cleanup_deferred;
}
/* Transmit buffer */
@@ -669,7 +670,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
if (txbuflen > 0) {
txbuf = devm_kzalloc(par->info->device, txbuflen, GFP_KERNEL);
if (!txbuf)
- goto release_framebuf;
+ goto cleanup_deferred;
par->txbuf.buf = txbuf;
par->txbuf.len = txbuflen;
}
@@ -691,6 +692,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
return info;
+cleanup_deferred:
+ fb_deferred_io_cleanup(info);
release_framebuf:
framebuffer_release(info);
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/2] staging: fbtft: cleanup fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-29 14:40 UTC (permalink / raw)
To: andy
Cc: Abdun Nihaal, dan.carpenter, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel
Fix a potential memory leak and cleanup error handling in
fbtft_framebuffer_alloc().
v3:
- Remove a redundant check before calling kfree
v2:
- Change the earlier patch to also handle the error code returned by
fb_deferred_io_init() and update Fixes tag to point to the commit that
introduced the memory allocation (which leads to leak).
- Add second patch to make the error handling order symmetric to
fbtft_framebuffer_release() and also remove managed allocation for
txbuf as suggested by Andy and Dan.
Link to v2: https://lore.kernel.org/linux-staging/cover.1751086324.git.abdun.nihaal@gmail.com/T/#md111471ddd69e6ddb0a6b98e565551ffbd791a34
Link to v1: https://lore.kernel.org/all/20250626172412.18355-1-abdun.nihaal@gmail.com/
Abdun Nihaal (2):
staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
staging: fbtft: cleanup error handling in fbtft_framebuffer_alloc()
drivers/staging/fbtft/fbtft-core.c | 38 +++++++++++++++++-------------
1 file changed, 21 insertions(+), 17 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v2 2/2] staging: fbtft: cleanup error handling in fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-29 14:23 UTC (permalink / raw)
To: Andy Shevchenko
Cc: andy, dan.carpenter, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel, Andy Shevchenko
In-Reply-To: <CAHp75VeSYesZuJ-NEfEAvaRepEUtdLmxGrYmthD1YkSg-bsK_g@mail.gmail.com>
On Sat, Jun 28, 2025 at 10:58:20PM +0300, Andy Shevchenko wrote:
> > + struct fbtft_par *par = info->par;
> > +
> > + if (par->txbuf.len > 0)
>
> Do we really need this check? If txbuf.buf is kept NULL (please, check
> this), the kfree() is NULL-aware.
I assumed that the par->txbuf.buf may be uninitialized, but I checked it
now and it uses kzalloc to allocate, so it must be NULL to begin with.
I'll remove the check, and send a v3.
Regards,
Nihaal
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Hans de Goede @ 2025-06-29 12:07 UTC (permalink / raw)
To: Krzysztof Kozlowski, Luca Weiss
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Javier Martinez Canillas, Helge Deller, linux-fbdev, dri-devel,
devicetree, linux-kernel
In-Reply-To: <1129bc60-f9cb-40be-9869-8ffa3b3c9748@kernel.org>
Hi Krzysztof,
On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote:
> On 27/06/2025 11:48, Luca Weiss wrote:
>> Hi Krzysztof,
>>
>> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote:
>>> On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
>>>> Document the interconnects property which is a list of interconnect
>>>> paths that is used by the framebuffer and therefore needs to be kept
>>>> alive when the framebuffer is being used.
>>>>
>>>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>> ---
>>>> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>>> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
>>>> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>>> @@ -79,6 +79,9 @@ properties:
>>>> power-domains:
>>>> description: List of power domains used by the framebuffer.
>>>>
>>>> + interconnects:
>>>> + description: List of interconnect paths used by the framebuffer.
>>>> +
>>>
>>> maxItems: 1, or this is not a simple FB anymore. Anything which needs
>>> some sort of resources in unknown way is not simple anymore. You need
>>> device specific bindings.
>>
>> The bindings support an arbitrary number of clocks, regulators,
>> power-domains. Why should I artificially limit the interconnects to only
>> one?
>
> And IMO they should not. Bindings are not supposed to be generic.
The simplefb binding is a binding to allow keeping the firmware, e.g.
uboot setup framebuffer alive to e.g. show a boot splash until
the native display-engine drive loads. Needing display-engine
specific bindings totally contradicts the whole goal of
It is generic by nature and I really do not see how clocks and
regulators are any different then interconnects here.
Note that we had a huge discussion about adding clock
and regulators to simplefb many years ago with pretty
much the same arguments against doing so. In the end it was
decided to add regulator and clocks support to the simplefb
bindings and non of the feared problems with e.g. ordening
of turning things on happened.
A big part of this is that the claiming of clks / regulators /
interconnects by the simplefb driver is there to keep things on,
so it happens before the kernel starts tuning off unused resources
IOW everything is already up and running and this really is about
avoiding turning things off.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 2/2] staging: fbtft: cleanup error handling in fbtft_framebuffer_alloc()
From: Andy Shevchenko @ 2025-06-28 19:58 UTC (permalink / raw)
To: Abdun Nihaal
Cc: andy, dan.carpenter, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel, Andy Shevchenko
In-Reply-To: <62320323049c72b6e3fda6fa7a55e080b29491e8.1751086324.git.abdun.nihaal@gmail.com>
On Sat, Jun 28, 2025 at 7:59 AM Abdun Nihaal <abdun.nihaal@gmail.com> wrote:
>
> The error handling in fbtft_framebuffer_alloc() mixes managed allocation
> and plain allocation, and performs error handling in an order different
> from the order in fbtft_framebuffer_release().
>
> Fix them by moving vmem allocation closer to where it is used, and using
> plain kzalloc() for txbuf allocation.
...
> + struct fbtft_par *par = info->par;
> +
> + if (par->txbuf.len > 0)
Do we really need this check? If txbuf.buf is kept NULL (please, check
this), the kfree() is NULL-aware.
> + kfree(par->txbuf.buf);
> fb_deferred_io_cleanup(info);
> vfree(info->screen_buffer);
> framebuffer_release(info);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Krzysztof Kozlowski @ 2025-06-28 11:50 UTC (permalink / raw)
To: Thomas Zimmermann, Luca Weiss
Cc: Hans de Goede, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Javier Martinez Canillas, Helge Deller, linux-fbdev, dri-devel,
devicetree, linux-kernel
In-Reply-To: <d8d85415-efc4-4a11-842e-23272cae29f7@suse.de>
On 27/06/2025 13:34, Thomas Zimmermann wrote:
> Hi
>
> Am 27.06.25 um 10:08 schrieb Krzysztof Kozlowski:
>> On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
>>> Document the interconnects property which is a list of interconnect
>>> paths that is used by the framebuffer and therefore needs to be kept
>>> alive when the framebuffer is being used.
>>>
>>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>>> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
>>> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> @@ -79,6 +79,9 @@ properties:
>>> power-domains:
>>> description: List of power domains used by the framebuffer.
>>>
>>> + interconnects:
>>> + description: List of interconnect paths used by the framebuffer.
>>> +
>> maxItems: 1, or this is not a simple FB anymore. Anything which needs
>> some sort of resources in unknown way is not simple anymore. You need
>> device specific bindings.
>
> In this context, 'simple' means that this device cannot change display
> modes or do graphics acceleration. The hardware itself is not
> necessarily simple. As Javier pointed out, it's initialized by firmware
If hardware is not simple, then it needs specific bindings.
> on the actual hardware. Think of 'VGA-for-ARM'. We need these resources
> to keep the display working.
I don't claim you do not need these resources. I claim device is not
simple thus does not suit rules for generic bindings. Generic bindings
are in general not allowed and we have them only for very, very simple
devices.
You say this is not simple device, so there you go - specific binding
for this complex (not-simple) device.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Krzysztof Kozlowski @ 2025-06-28 11:49 UTC (permalink / raw)
To: Luca Weiss
Cc: Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Javier Martinez Canillas,
Helge Deller, linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <DAX7ZB27SBPV.2Y0I09TVSF3TT@fairphone.com>
On 27/06/2025 11:48, Luca Weiss wrote:
> Hi Krzysztof,
>
> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote:
>> On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
>>> Document the interconnects property which is a list of interconnect
>>> paths that is used by the framebuffer and therefore needs to be kept
>>> alive when the framebuffer is being used.
>>>
>>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>>> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
>>> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> @@ -79,6 +79,9 @@ properties:
>>> power-domains:
>>> description: List of power domains used by the framebuffer.
>>>
>>> + interconnects:
>>> + description: List of interconnect paths used by the framebuffer.
>>> +
>>
>> maxItems: 1, or this is not a simple FB anymore. Anything which needs
>> some sort of resources in unknown way is not simple anymore. You need
>> device specific bindings.
>
> The bindings support an arbitrary number of clocks, regulators,
> power-domains. Why should I artificially limit the interconnects to only
> one?
And IMO they should not. Bindings are not supposed to be generic.
>
> The driver code also has that support added in this series.
That's not the problem here.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: fix all checkpatch warnings in .c and .h files
From: Greg KH @ 2025-06-28 8:29 UTC (permalink / raw)
To: Manish Kumar
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20250628082305.20847-1-manish1588@gmail.com>
On Sat, Jun 28, 2025 at 01:53:05PM +0530, Manish Kumar wrote:
> This patch resolves all checkpatch.pl --strict warnings in the
> sm750fb driver source files, including both C and header files.
>
> Changes include:
> - Replacing CamelCase identifiers with snake_case
> - Avoiding chained assignments
> - Fixing indentation, spacing, and alignment issues
> - Updating function declarations and comment styles
> - Making code conform to kernel coding style
>
> No functional changes.
>
> Signed-off-by: Manish Kumar <manish1588@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.c | 90 ++++++++++---------
> drivers/staging/sm750fb/sm750.h | 32 +++----
> drivers/staging/sm750fb/sm750_accel.c | 120 +++++++++++++-------------
> drivers/staging/sm750fb/sm750_hw.c | 24 +++---
> 4 files changed, 135 insertions(+), 131 deletions(-)
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- Your patch did many different things all at once, making it difficult
to review. All Linux kernel patches need to only do one thing at a
time. If you need to do multiple things (such as clean up all coding
style issues in a file/driver), do it in a sequence of patches, each
one doing only one thing. This will make it easier to review the
patches to ensure that they are correct, and to help alleviate any
merge issues that larger patches can cause.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* [PATCH] staging: sm750fb: fix all checkpatch warnings in .c and .h files
From: Manish Kumar @ 2025-06-28 8:23 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, linux-staging, linux-kernel, Manish Kumar
This patch resolves all checkpatch.pl --strict warnings in the
sm750fb driver source files, including both C and header files.
Changes include:
- Replacing CamelCase identifiers with snake_case
- Avoiding chained assignments
- Fixing indentation, spacing, and alignment issues
- Updating function declarations and comment styles
- Making code conform to kernel coding style
No functional changes.
Signed-off-by: Manish Kumar <manish1588@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 90 ++++++++++---------
drivers/staging/sm750fb/sm750.h | 32 +++----
drivers/staging/sm750fb/sm750_accel.c | 120 +++++++++++++-------------
drivers/staging/sm750fb/sm750_hw.c | 24 +++---
4 files changed, 135 insertions(+), 131 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index e77ad73f0db1..8794195a9594 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -33,7 +33,7 @@
static int g_hwcursor = 1;
static int g_noaccel;
static int g_nomtrr;
-static const char * const g_fbmode[] = {NULL, NULL};
+static const char *g_fbmode[] = {NULL, NULL};
static const char *g_def_fbmode = "1024x768-32@60";
static char *g_settings;
static int g_dualview;
@@ -160,7 +160,7 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
{
struct lynxfb_par *par;
struct sm750_dev *sm750_dev;
- unsigned int base, pitch, Bpp, rop;
+ unsigned int base, pitch, bpp, rop;
u32 color;
if (info->state != FBINFO_STATE_RUNNING)
@@ -175,9 +175,9 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
*/
base = par->crtc.o_screen;
pitch = info->fix.line_length;
- Bpp = info->var.bits_per_pixel >> 3;
+ bpp = info->var.bits_per_pixel >> 3;
- color = (Bpp == 1) ? region->color :
+ color = (bpp == 1) ? region->color :
((u32 *)info->pseudo_palette)[region->color];
rop = (region->rop != ROP_COPY) ? HW_ROP2_XOR : HW_ROP2_COPY;
@@ -190,7 +190,7 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
spin_lock(&sm750_dev->slock);
sm750_dev->accel.de_fillrect(&sm750_dev->accel,
- base, pitch, Bpp,
+ base, pitch, bpp,
region->dx, region->dy,
region->width, region->height,
color, rop);
@@ -202,7 +202,7 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
{
struct lynxfb_par *par;
struct sm750_dev *sm750_dev;
- unsigned int base, pitch, Bpp;
+ unsigned int base, pitch, bpp;
par = info->par;
sm750_dev = par->dev;
@@ -213,7 +213,7 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
*/
base = par->crtc.o_screen;
pitch = info->fix.line_length;
- Bpp = info->var.bits_per_pixel >> 3;
+ bpp = info->var.bits_per_pixel >> 3;
/*
* If not use spin_lock, system will die if user load driver
@@ -225,7 +225,7 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
sm750_dev->accel.de_copyarea(&sm750_dev->accel,
base, pitch, region->sx, region->sy,
- base, pitch, Bpp, region->dx, region->dy,
+ base, pitch, bpp, region->dx, region->dy,
region->width, region->height,
HW_ROP2_COPY);
spin_unlock(&sm750_dev->slock);
@@ -234,7 +234,7 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
static void lynxfb_ops_imageblit(struct fb_info *info,
const struct fb_image *image)
{
- unsigned int base, pitch, Bpp;
+ unsigned int base, pitch, bpp;
unsigned int fgcol, bgcol;
struct lynxfb_par *par;
struct sm750_dev *sm750_dev;
@@ -247,7 +247,7 @@ static void lynxfb_ops_imageblit(struct fb_info *info,
*/
base = par->crtc.o_screen;
pitch = info->fix.line_length;
- Bpp = info->var.bits_per_pixel >> 3;
+ bpp = info->var.bits_per_pixel >> 3;
/* TODO: Implement hardware acceleration for image->depth > 1 */
if (image->depth != 1) {
@@ -274,7 +274,7 @@ static void lynxfb_ops_imageblit(struct fb_info *info,
sm750_dev->accel.de_imageblit(&sm750_dev->accel,
image->data, image->width >> 3, 0,
- base, pitch, Bpp,
+ base, pitch, bpp,
image->dx, image->dy,
image->width, image->height,
fgcol, bgcol, HW_ROP2_COPY);
@@ -526,6 +526,7 @@ static int lynxfb_ops_setcolreg(unsigned int regno,
struct lynxfb_crtc *crtc;
struct fb_var_screeninfo *var;
int ret;
+ int gray;
par = info->par;
crtc = &par->crtc;
@@ -538,7 +539,10 @@ static int lynxfb_ops_setcolreg(unsigned int regno,
}
if (info->var.grayscale)
- red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+ gray = (red * 77 + green * 151 + blue * 28) >> 8;
+ red = gray;
+ green = gray;
+ blue = gray;
if (var->bits_per_pixel == 8 &&
info->fix.visual == FB_VISUAL_PSEUDOCOLOR) {
@@ -577,7 +581,7 @@ static int lynxfb_ops_blank(int blank, struct fb_info *info)
pr_debug("blank = %d.\n", blank);
par = info->par;
output = &par->output;
- return output->proc_setBLANK(output, blank);
+ return output->proc_set_BLANK(output, blank);
}
static int sm750fb_set_drv(struct lynxfb_par *par)
@@ -598,14 +602,14 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
crtc->vidmem_size >>= 1;
/* setup crtc and output member */
- sm750_dev->hwCursor = g_hwcursor;
+ sm750_dev->hw_cursor = g_hwcursor;
crtc->line_pad = 16;
crtc->xpanstep = 8;
crtc->ypanstep = 1;
crtc->ywrapstep = 0;
- output->proc_setBLANK = (sm750_dev->revid == SM750LE_REVISION_ID) ?
+ output->proc_set_BLANK = (sm750_dev->revid == SM750LE_REVISION_ID) ?
hw_sm750le_set_blank : hw_sm750_set_blank;
/* chip specific phase */
sm750_dev->accel.de_wait = (sm750_dev->revid == SM750LE_REVISION_ID) ?
@@ -615,27 +619,27 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
output->paths = sm750_pnc;
crtc->channel = sm750_primary;
crtc->o_screen = 0;
- crtc->v_screen = sm750_dev->pvMem;
+ crtc->v_screen = sm750_dev->pv_mem;
pr_info("use simul primary mode\n");
break;
case sm750_simul_sec:
output->paths = sm750_pnc;
crtc->channel = sm750_secondary;
crtc->o_screen = 0;
- crtc->v_screen = sm750_dev->pvMem;
+ crtc->v_screen = sm750_dev->pv_mem;
break;
case sm750_dual_normal:
if (par->index == 0) {
output->paths = sm750_panel;
crtc->channel = sm750_primary;
crtc->o_screen = 0;
- crtc->v_screen = sm750_dev->pvMem;
+ crtc->v_screen = sm750_dev->pv_mem;
} else {
output->paths = sm750_crt;
crtc->channel = sm750_secondary;
/* not consider of padding stuffs for o_screen,need fix */
crtc->o_screen = sm750_dev->vidmem_size >> 1;
- crtc->v_screen = sm750_dev->pvMem + crtc->o_screen;
+ crtc->v_screen = sm750_dev->pv_mem + crtc->o_screen;
}
break;
case sm750_dual_swap:
@@ -643,7 +647,7 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
output->paths = sm750_panel;
crtc->channel = sm750_secondary;
crtc->o_screen = 0;
- crtc->v_screen = sm750_dev->pvMem;
+ crtc->v_screen = sm750_dev->pv_mem;
} else {
output->paths = sm750_crt;
crtc->channel = sm750_primary;
@@ -651,7 +655,7 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
* need fix
*/
crtc->o_screen = sm750_dev->vidmem_size >> 1;
- crtc->v_screen = sm750_dev->pvMem + crtc->o_screen;
+ crtc->v_screen = sm750_dev->pv_mem + crtc->o_screen;
}
break;
default:
@@ -731,7 +735,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
"kernel HELPERS prepared vesa_modes",
};
- static const char *fixId[2] = {
+ static const char *fix_id[2] = {
"sm750_fb1", "sm750_fb2",
};
@@ -755,14 +759,14 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
* must be set after crtc member initialized
*/
crtc->cursor.offset = crtc->o_screen + crtc->vidmem_size - 1024;
- crtc->cursor.mmio = sm750_dev->pvReg +
+ crtc->cursor.mmio = sm750_dev->pv_reg +
0x800f0 + (int)crtc->channel * 0x140;
pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
crtc->cursor.max_h = 64;
crtc->cursor.max_w = 64;
crtc->cursor.size = crtc->cursor.max_h * crtc->cursor.max_w * 2 / 8;
- crtc->cursor.vstart = sm750_dev->pvMem + crtc->cursor.offset;
+ crtc->cursor.vstart = sm750_dev->pv_mem + crtc->cursor.offset;
memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);
if (!g_hwcursor)
@@ -853,7 +857,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
fix->ywrapstep = crtc->ywrapstep;
fix->accel = FB_ACCEL_SMI;
- strscpy(fix->id, fixId[index], sizeof(fix->id));
+ strscpy(fix->id, fix_id[index], sizeof(fix->id));
fix->smem_start = crtc->o_screen + sm750_dev->vidmem_start;
pr_info("fix->smem_start = %lx\n", fix->smem_start);
@@ -909,12 +913,12 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
swap = 0;
- sm750_dev->initParm.chip_clk = 0;
- sm750_dev->initParm.mem_clk = 0;
- sm750_dev->initParm.master_clk = 0;
- sm750_dev->initParm.powerMode = 0;
- sm750_dev->initParm.setAllEngOff = 0;
- sm750_dev->initParm.resetMemory = 1;
+ sm750_dev->init_parm.chip_clk = 0;
+ sm750_dev->init_parm.mem_clk = 0;
+ sm750_dev->init_parm.master_clk = 0;
+ sm750_dev->init_parm.power_mode = 0;
+ sm750_dev->init_parm.set_all_eng_off = 0;
+ sm750_dev->init_parm.reset_memory = 1;
/* defaultly turn g_hwcursor on for both view */
g_hwcursor = 3;
@@ -933,11 +937,11 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
} else if (!strncmp(opt, "nocrt", strlen("nocrt"))) {
sm750_dev->nocrt = 1;
} else if (!strncmp(opt, "36bit", strlen("36bit"))) {
- sm750_dev->pnltype = sm750_doubleTFT;
+ sm750_dev->pnl_type = sm750_double_TFT;
} else if (!strncmp(opt, "18bit", strlen("18bit"))) {
- sm750_dev->pnltype = sm750_dualTFT;
+ sm750_dev->pnl_type = sm750_dual_TFT;
} else if (!strncmp(opt, "24bit", strlen("24bit"))) {
- sm750_dev->pnltype = sm750_24TFT;
+ sm750_dev->pnl_type = sm750_24TFT;
} else if (!strncmp(opt, "nohwc0", strlen("nohwc0"))) {
g_hwcursor &= ~0x1;
} else if (!strncmp(opt, "nohwc1", strlen("nohwc1"))) {
@@ -963,20 +967,20 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, char *src)
if (sm750_dev->revid != SM750LE_REVISION_ID) {
if (sm750_dev->fb_count > 1) {
if (swap)
- sm750_dev->dataflow = sm750_dual_swap;
+ sm750_dev->data_flow = sm750_dual_swap;
else
- sm750_dev->dataflow = sm750_dual_normal;
+ sm750_dev->data_flow = sm750_dual_normal;
} else {
if (swap)
- sm750_dev->dataflow = sm750_simul_sec;
+ sm750_dev->data_flow = sm750_simul_sec;
else
- sm750_dev->dataflow = sm750_simul_pri;
+ sm750_dev->data_flow = sm750_simul_pri;
}
} else {
/* SM750LE only have one crt channel */
- sm750_dev->dataflow = sm750_simul_sec;
+ sm750_dev->data_flow = sm750_simul_sec;
/* sm750le do not have complex attributes */
- sm750_dev->nocrt = 0;
+ sm750_dev->no_crt = 0;
}
}
@@ -1081,7 +1085,7 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
sm750_dev->mtrr.vram = arch_phys_wc_add(sm750_dev->vidmem_start,
sm750_dev->vidmem_size);
- memset_io(sm750_dev->pvMem, 0, sm750_dev->vidmem_size);
+ memset_io(sm750_dev->pv_mem, 0, sm750_dev->vidmem_size);
pci_set_drvdata(pdev, sm750_dev);
@@ -1112,8 +1116,8 @@ static void lynxfb_pci_remove(struct pci_dev *pdev)
sm750fb_framebuffer_release(sm750_dev);
arch_phys_wc_del(sm750_dev->mtrr.vram);
- iounmap(sm750_dev->pvReg);
- iounmap(sm750_dev->pvMem);
+ iounmap(sm750_dev->pv_reg);
+ iounmap(sm750_dev->pv_mem);
kfree(g_settings);
}
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index 9cf8b3d30aac..9ae21cebc669 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -13,9 +13,9 @@
#endif
enum sm750_pnltype {
- sm750_24TFT = 0, /* 24bit tft */
- sm750_dualTFT = 2, /* dual 18 bit tft */
- sm750_doubleTFT = 1, /* 36 bit double pixel tft */
+ sm750_24_TFT = 0, /* 24bit tft */
+ sm750_dual_TFT = 2, /* dual 18 bit tft */
+ sm750_double_TFT = 1, /* 36 bit double pixel tft */
};
/* vga channel is not concerned */
@@ -39,20 +39,20 @@ enum sm750_path {
};
struct init_status {
- ushort powerMode;
+ ushort power_mode;
/* below three clocks are in unit of MHZ*/
ushort chip_clk;
ushort mem_clk;
ushort master_clk;
- ushort setAllEngOff;
- ushort resetMemory;
+ ushort set_all_eng_off;
+ ushort reset_memory;
};
struct lynx_accel {
/* base virtual address of DPR registers */
- volatile unsigned char __iomem *dprBase;
+ volatile unsigned char __iomem *dpr_base;
/* base virtual address of de data port */
- volatile unsigned char __iomem *dpPortBase;
+ volatile unsigned char __iomem *dp_port_base;
/* function pointers */
void (*de_init)(struct lynx_accel *accel);
@@ -97,15 +97,15 @@ struct sm750_dev {
unsigned long vidreg_start;
__u32 vidmem_size;
__u32 vidreg_size;
- void __iomem *pvReg;
- unsigned char __iomem *pvMem;
+ void __iomem *pv_reg;
+ unsigned char __iomem *pv_mem;
/* locks*/
spinlock_t slock;
- struct init_status initParm;
- enum sm750_pnltype pnltype;
- enum sm750_dataflow dataflow;
- int nocrt;
+ struct init_status init_parm;
+ enum sm750_pnltype pnl_type;
+ enum sm750_dataflow data_flow;
+ int no_crt;
/*
* 0: no hardware cursor
@@ -113,7 +113,7 @@ struct sm750_dev {
* 2: secondary crtc hw cursor enabled
* 3: both ctrc hw cursor enabled
*/
- int hwCursor;
+ int hw_cursor;
};
struct lynx_cursor {
@@ -170,7 +170,7 @@ struct lynxfb_output {
*/
void *priv;
- int (*proc_setBLANK)(struct lynxfb_output *output, int blank);
+ int (*proc_set_BLANK)(struct lynxfb_output *output, int blank);
};
struct lynxfb_par {
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 44b9e3fe3a41..91a26f5f0e14 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -17,19 +17,19 @@
#include "sm750.h"
#include "sm750_accel.h"
-static inline void write_dpr(struct lynx_accel *accel, int offset, u32 regValue)
+static inline void write_dpr(struct lynx_accel *accel, int offset, u32 reg_value)
{
- writel(regValue, accel->dprBase + offset);
+ writel(reg_value, accel->dpr_base + offset);
}
static inline u32 read_dpr(struct lynx_accel *accel, int offset)
{
- return readl(accel->dprBase + offset);
+ return readl(accel->dpr_base + offset);
}
-static inline void write_dpPort(struct lynx_accel *accel, u32 data)
+static inline void write_dp_port(struct lynx_accel *accel, u32 data)
{
- writel(data, accel->dpPortBase);
+ writel(data, accel->dp_port_base);
}
void sm750_hw_de_init(struct lynx_accel *accel)
@@ -85,11 +85,11 @@ void sm750_hw_set2dformat(struct lynx_accel *accel, int fmt)
}
int sm750_hw_fillrect(struct lynx_accel *accel,
- u32 base, u32 pitch, u32 Bpp,
+ u32 base, u32 pitch, u32 bpp,
u32 x, u32 y, u32 width, u32 height,
u32 color, u32 rop)
{
- u32 deCtrl;
+ u32 de_ctrl;
if (accel->de_wait() != 0) {
/*
@@ -102,14 +102,14 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
write_dpr(accel, DE_WINDOW_DESTINATION_BASE, base); /* dpr40 */
write_dpr(accel, DE_PITCH,
- ((pitch / Bpp << DE_PITCH_DESTINATION_SHIFT) &
+ ((pitch / bpp << DE_PITCH_DESTINATION_SHIFT) &
DE_PITCH_DESTINATION_MASK) |
- (pitch / Bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
+ (pitch / bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
write_dpr(accel, DE_WINDOW_WIDTH,
- ((pitch / Bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
+ ((pitch / bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
DE_WINDOW_WIDTH_DST_MASK) |
- (pitch / Bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr44 */
+ (pitch / bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr44 */
write_dpr(accel, DE_FOREGROUND, color); /* DPR14 */
@@ -121,11 +121,11 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
((width << DE_DIMENSION_X_SHIFT) & DE_DIMENSION_X_MASK) |
(height & DE_DIMENSION_Y_ET_MASK)); /* dpr8 */
- deCtrl = DE_CONTROL_STATUS | DE_CONTROL_LAST_PIXEL |
+ de_ctrl = DE_CONTROL_STATUS | DE_CONTROL_LAST_PIXEL |
DE_CONTROL_COMMAND_RECTANGLE_FILL | DE_CONTROL_ROP_SELECT |
(rop & DE_CONTROL_ROP_MASK); /* dpr0xc */
- write_dpr(accel, DE_CONTROL, deCtrl);
+ write_dpr(accel, DE_CONTROL, de_ctrl);
return 0;
}
@@ -146,21 +146,21 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
* @rop2: ROP value
*/
int sm750_hw_copyarea(struct lynx_accel *accel,
- unsigned int sBase, unsigned int sPitch,
+ unsigned int s_base, unsigned int s_pitch,
unsigned int sx, unsigned int sy,
- unsigned int dBase, unsigned int dPitch,
- unsigned int Bpp, unsigned int dx, unsigned int dy,
+ unsigned int d_base, unsigned int d_pitch,
+ unsigned int bpp, unsigned int dx, unsigned int dy,
unsigned int width, unsigned int height,
unsigned int rop2)
{
- unsigned int nDirection, de_ctrl;
+ unsigned int n_direction, de_ctrl;
- nDirection = LEFT_TO_RIGHT;
+ n_direction = LEFT_TO_RIGHT;
/* Direction of ROP2 operation: 1 = Left to Right, (-1) = Right to Left */
de_ctrl = 0;
/* If source and destination are the same surface, need to check for overlay cases */
- if (sBase == dBase && sPitch == dPitch) {
+ if (s_base == d_base && s_pitch == d_pitch) {
/* Determine direction of operation */
if (sy < dy) {
/* +----------+
@@ -173,7 +173,7 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* +----------+
*/
- nDirection = BOTTOM_TO_TOP;
+ n_direction = BOTTOM_TO_TOP;
} else if (sy > dy) {
/* +----------+
* |D |
@@ -185,7 +185,7 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* +----------+
*/
- nDirection = TOP_TO_BOTTOM;
+ n_direction = TOP_TO_BOTTOM;
} else {
/* sy == dy */
@@ -198,7 +198,7 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* +------+---+------+
*/
- nDirection = RIGHT_TO_LEFT;
+ n_direction = RIGHT_TO_LEFT;
} else {
/* sx > dx */
@@ -210,12 +210,12 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* +------+---+------+
*/
- nDirection = LEFT_TO_RIGHT;
+ n_direction = LEFT_TO_RIGHT;
}
}
}
- if ((nDirection == BOTTOM_TO_TOP) || (nDirection == RIGHT_TO_LEFT)) {
+ if ((n_direction == BOTTOM_TO_TOP) || (n_direction == RIGHT_TO_LEFT)) {
sx += width - 1;
sy += height - 1;
dx += width - 1;
@@ -234,14 +234,14 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* It is an address offset (128 bit aligned)
* from the beginning of frame buffer.
*/
- write_dpr(accel, DE_WINDOW_SOURCE_BASE, sBase); /* dpr40 */
+ write_dpr(accel, DE_WINDOW_SOURCE_BASE, s_base); /* dpr40 */
/*
* 2D Destination Base.
* It is an address offset (128 bit aligned)
* from the beginning of frame buffer.
*/
- write_dpr(accel, DE_WINDOW_DESTINATION_BASE, dBase); /* dpr44 */
+ write_dpr(accel, DE_WINDOW_DESTINATION_BASE, d_base); /* dpr44 */
/*
* Program pitch (distance between the 1st points of two adjacent lines).
@@ -249,9 +249,9 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* pixel values. Need Byte to pixel conversion.
*/
write_dpr(accel, DE_PITCH,
- ((dPitch / Bpp << DE_PITCH_DESTINATION_SHIFT) &
+ ((d_pitch / bpp << DE_PITCH_DESTINATION_SHIFT) &
DE_PITCH_DESTINATION_MASK) |
- (sPitch / Bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
+ (s_pitch / bpp & DE_PITCH_SOURCE_MASK)); /* dpr10 */
/*
* Screen Window width in Pixels.
@@ -259,9 +259,9 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
* for a given point.
*/
write_dpr(accel, DE_WINDOW_WIDTH,
- ((dPitch / Bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
+ ((d_pitch / bpp << DE_WINDOW_WIDTH_DST_SHIFT) &
DE_WINDOW_WIDTH_DST_MASK) |
- (sPitch / Bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr3c */
+ (s_pitch / bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr3c */
if (accel->de_wait() != 0)
return -1;
@@ -277,14 +277,14 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
(height & DE_DIMENSION_Y_ET_MASK)); /* dpr08 */
de_ctrl = (rop2 & DE_CONTROL_ROP_MASK) | DE_CONTROL_ROP_SELECT |
- ((nDirection == RIGHT_TO_LEFT) ? DE_CONTROL_DIRECTION : 0) |
+ ((n_direction == RIGHT_TO_LEFT) ? DE_CONTROL_DIRECTION : 0) |
DE_CONTROL_COMMAND_BITBLT | DE_CONTROL_STATUS;
write_dpr(accel, DE_CONTROL, de_ctrl); /* dpr0c */
return 0;
}
-static unsigned int deGetTransparency(struct lynx_accel *accel)
+static unsigned int de_get_transparency(struct lynx_accel *accel)
{
unsigned int de_ctrl;
@@ -315,22 +315,22 @@ static unsigned int deGetTransparency(struct lynx_accel *accel)
* @bColor: Background color (corresponding to a 0 in the monochrome data
* @rop2: ROP value
*/
-int sm750_hw_imageblit(struct lynx_accel *accel, const char *pSrcbuf,
- u32 srcDelta, u32 startBit, u32 dBase, u32 dPitch,
- u32 bytePerPixel, u32 dx, u32 dy, u32 width,
- u32 height, u32 fColor, u32 bColor, u32 rop2)
+int sm750_hw_imageblit(struct lynx_accel *accel, const char *p_src_buf,
+ u32 src_delta, u32 start_bit, u32 d_base, u32 d_pitch,
+ u32 byte_per_pixel, u32 dx, u32 dy, u32 width,
+ u32 height, u32 f_color, u32 b_color, u32 rop2)
{
- unsigned int ulBytesPerScan;
- unsigned int ul4BytesPerScan;
- unsigned int ulBytesRemain;
+ unsigned int ul_bytes_per_scan;
+ unsigned int ul_4bytes_per_scan;
+ unsigned int ul_bytes_remain;
unsigned int de_ctrl = 0;
- unsigned char ajRemain[4];
+ unsigned char aj_remain[4];
int i, j;
- startBit &= 7; /* Just make sure the start bit is within legal range */
- ulBytesPerScan = (width + startBit + 7) / 8;
- ul4BytesPerScan = ulBytesPerScan & ~3;
- ulBytesRemain = ulBytesPerScan & 3;
+ start_bit &= 7; /* Just make sure the start bit is within legal range */
+ ul_bytes_per_scan = (width + start_bit + 7) / 8;
+ ul_4bytes_per_scan = ul_bytes_per_scan & ~3;
+ ul_bytes_remain = ul_bytes_per_scan & 3;
if (accel->de_wait() != 0)
return -1;
@@ -345,7 +345,7 @@ int sm750_hw_imageblit(struct lynx_accel *accel, const char *pSrcbuf,
* It is an address offset (128 bit aligned)
* from the beginning of frame buffer.
*/
- write_dpr(accel, DE_WINDOW_DESTINATION_BASE, dBase);
+ write_dpr(accel, DE_WINDOW_DESTINATION_BASE, d_base);
/*
* Program pitch (distance between the 1st points of two adjacent
@@ -353,9 +353,9 @@ int sm750_hw_imageblit(struct lynx_accel *accel, const char *pSrcbuf,
* register uses pixel values. Need Byte to pixel conversion.
*/
write_dpr(accel, DE_PITCH,
- ((dPitch / bytePerPixel << DE_PITCH_DESTINATION_SHIFT) &
+ ((d_pitch / byte_per_pixel << DE_PITCH_DESTINATION_SHIFT) &
DE_PITCH_DESTINATION_MASK) |
- (dPitch / bytePerPixel & DE_PITCH_SOURCE_MASK)); /* dpr10 */
+ (d_pitch / byte_per_pixel & DE_PITCH_SOURCE_MASK)); /* dpr10 */
/*
* Screen Window width in Pixels.
@@ -363,9 +363,9 @@ int sm750_hw_imageblit(struct lynx_accel *accel, const char *pSrcbuf,
* in frame buffer for a given point.
*/
write_dpr(accel, DE_WINDOW_WIDTH,
- ((dPitch / bytePerPixel << DE_WINDOW_WIDTH_DST_SHIFT) &
+ ((d_pitch / byte_per_pixel << DE_WINDOW_WIDTH_DST_SHIFT) &
DE_WINDOW_WIDTH_DST_MASK) |
- (dPitch / bytePerPixel & DE_WINDOW_WIDTH_SRC_MASK));
+ (d_pitch / byte_per_pixel & DE_WINDOW_WIDTH_SRC_MASK));
/*
* Note: For 2D Source in Host Write, only X_K1_MONO field is needed,
@@ -373,7 +373,7 @@ int sm750_hw_imageblit(struct lynx_accel *accel, const char *pSrcbuf,
* For mono bitmap, use startBit for X_K1.
*/
write_dpr(accel, DE_SOURCE,
- (startBit << DE_SOURCE_X_K1_SHIFT) &
+ (start_bit << DE_SOURCE_X_K1_SHIFT) &
DE_SOURCE_X_K1_MONO_MASK); /* dpr00 */
write_dpr(accel, DE_DESTINATION,
@@ -384,28 +384,28 @@ int sm750_hw_imageblit(struct lynx_accel *accel, const char *pSrcbuf,
((width << DE_DIMENSION_X_SHIFT) & DE_DIMENSION_X_MASK) |
(height & DE_DIMENSION_Y_ET_MASK)); /* dpr08 */
- write_dpr(accel, DE_FOREGROUND, fColor);
- write_dpr(accel, DE_BACKGROUND, bColor);
+ write_dpr(accel, DE_FOREGROUND, f_color);
+ write_dpr(accel, DE_BACKGROUND, b_color);
de_ctrl = (rop2 & DE_CONTROL_ROP_MASK) |
DE_CONTROL_ROP_SELECT | DE_CONTROL_COMMAND_HOST_WRITE |
DE_CONTROL_HOST | DE_CONTROL_STATUS;
- write_dpr(accel, DE_CONTROL, de_ctrl | deGetTransparency(accel));
+ write_dpr(accel, DE_CONTROL, de_ctrl | de_get_transparency(accel));
/* Write MONO data (line by line) to 2D Engine data port */
for (i = 0; i < height; i++) {
/* For each line, send the data in chunks of 4 bytes */
- for (j = 0; j < (ul4BytesPerScan / 4); j++)
- write_dpPort(accel, *(unsigned int *)(pSrcbuf + (j * 4)));
+ for (j = 0; j < (ul_4bytes_per_scan / 4); j++)
+ write_dp_port(accel, *(unsigned int *)(p_src_buf + (j * 4)));
- if (ulBytesRemain) {
- memcpy(ajRemain, pSrcbuf + ul4BytesPerScan,
- ulBytesRemain);
- write_dpPort(accel, *(unsigned int *)ajRemain);
+ if (ul_bytes_remain) {
+ memcpy(aj_remain, p_src_buf + ul_4bytes_per_scan,
+ ul_bytes_remain);
+ write_dp_port(accel, *(unsigned int *)aj_remain);
}
- pSrcbuf += srcDelta;
+ p_src_buf += src_delta;
}
return 0;
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 7119b67efe11..ce6a0d2ebbb5 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -49,19 +49,19 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
}
/* now map mmio and vidmem */
- sm750_dev->pvReg =
+ sm750_dev->pv_reg =
ioremap(sm750_dev->vidreg_start, sm750_dev->vidreg_size);
- if (!sm750_dev->pvReg) {
+ if (!sm750_dev->pv_reg) {
pr_err("mmio failed\n");
ret = -EFAULT;
goto exit;
}
- pr_info("mmio virtual addr = %p\n", sm750_dev->pvReg);
+ pr_info("mmio virtual addr = %p\n", sm750_dev->pv_reg);
- sm750_dev->accel.dprBase = sm750_dev->pvReg + DE_BASE_ADDR_TYPE1;
- sm750_dev->accel.dpPortBase = sm750_dev->pvReg + DE_PORT_ADDR_TYPE1;
+ sm750_dev->accel.dpr_base = sm750_dev->pv_reg + DE_BASE_ADDR_TYPE1;
+ sm750_dev->accel.dp_port_base = sm750_dev->pv_reg + DE_PORT_ADDR_TYPE1;
- mmio750 = sm750_dev->pvReg;
+ mmio750 = sm750_dev->pv_reg;
sm750_set_chip_type(sm750_dev->devid, sm750_dev->revid);
sm750_dev->vidmem_start = pci_resource_start(pdev, 0);
@@ -76,15 +76,15 @@ int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
sm750_dev->vidmem_start, sm750_dev->vidmem_size);
/* reserve the vidmem space of smi adaptor */
- sm750_dev->pvMem =
+ sm750_dev->pv_mem =
ioremap_wc(sm750_dev->vidmem_start, sm750_dev->vidmem_size);
- if (!sm750_dev->pvMem) {
- iounmap(sm750_dev->pvReg);
+ if (!sm750_dev->pv_mem) {
+ iounmap(sm750_dev->pv_reg);
pr_err("Map video memory failed\n");
ret = -EFAULT;
goto exit;
}
- pr_info("video memory vaddr = %p\n", sm750_dev->pvMem);
+ pr_info("video memory vaddr = %p\n", sm750_dev->pv_mem);
exit:
return ret;
}
@@ -93,7 +93,7 @@ int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
{
struct init_status *parm;
- parm = &sm750_dev->initParm;
+ parm = &sm750_dev->init_parm;
if (parm->chip_clk == 0)
parm->chip_clk = (sm750_get_chip_type() == SM750LE) ?
DEFAULT_SM750LE_CHIP_CLOCK :
@@ -104,7 +104,7 @@ int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev)
if (parm->master_clk == 0)
parm->master_clk = parm->chip_clk / 3;
- ddk750_init_hw((struct initchip_param *)&sm750_dev->initParm);
+ ddk750_init_hw((struct initchip_param *)&sm750_dev->init_parm);
/* for sm718, open pci burst */
if (sm750_dev->devid == 0x718) {
poke32(SYSTEM_CTRL,
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/2] staging: fbtft: cleanup error handling in fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-28 4:59 UTC (permalink / raw)
To: andy
Cc: Abdun Nihaal, dan.carpenter, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel, Andy Shevchenko
In-Reply-To: <cover.1751086324.git.abdun.nihaal@gmail.com>
The error handling in fbtft_framebuffer_alloc() mixes managed allocation
and plain allocation, and performs error handling in an order different
from the order in fbtft_framebuffer_release().
Fix them by moving vmem allocation closer to where it is used, and using
plain kzalloc() for txbuf allocation.
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Suggested-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
---
Newly added in v2
drivers/staging/fbtft/fbtft-core.c | 32 ++++++++++++++++--------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8538b6bab6a5..f6a147cf0717 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -568,18 +568,13 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
height = display->height;
}
- vmem_size = display->width * display->height * bpp / 8;
- vmem = vzalloc(vmem_size);
- if (!vmem)
- goto alloc_fail;
-
fbdefio = devm_kzalloc(dev, sizeof(struct fb_deferred_io), GFP_KERNEL);
if (!fbdefio)
- goto alloc_fail;
+ return NULL;
buf = devm_kzalloc(dev, 128, GFP_KERNEL);
if (!buf)
- goto alloc_fail;
+ return NULL;
if (display->gamma_num && display->gamma_len) {
gamma_curves = devm_kcalloc(dev,
@@ -588,12 +583,17 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
sizeof(gamma_curves[0]),
GFP_KERNEL);
if (!gamma_curves)
- goto alloc_fail;
+ return NULL;
}
info = framebuffer_alloc(sizeof(struct fbtft_par), dev);
if (!info)
- goto alloc_fail;
+ return NULL;
+
+ vmem_size = display->width * display->height * bpp / 8;
+ vmem = vzalloc(vmem_size);
+ if (!vmem)
+ goto release_framebuf;
info->screen_buffer = vmem;
info->fbops = &fbtft_ops;
@@ -613,7 +613,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
info->fix.accel = FB_ACCEL_NONE;
info->fix.smem_len = vmem_size;
if (fb_deferred_io_init(info))
- goto release_framebuf;
+ goto release_screen_buffer;
info->var.rotate = pdata->rotate;
info->var.xres = width;
@@ -668,7 +668,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
#endif
if (txbuflen > 0) {
- txbuf = devm_kzalloc(par->info->device, txbuflen, GFP_KERNEL);
+ txbuf = kzalloc(txbuflen, GFP_KERNEL);
if (!txbuf)
goto cleanup_deferred;
par->txbuf.buf = txbuf;
@@ -694,12 +694,10 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
cleanup_deferred:
fb_deferred_io_cleanup(info);
+release_screen_buffer:
+ vfree(info->screen_buffer);
release_framebuf:
framebuffer_release(info);
-
-alloc_fail:
- vfree(vmem);
-
return NULL;
}
EXPORT_SYMBOL(fbtft_framebuffer_alloc);
@@ -712,6 +710,10 @@ EXPORT_SYMBOL(fbtft_framebuffer_alloc);
*/
void fbtft_framebuffer_release(struct fb_info *info)
{
+ struct fbtft_par *par = info->par;
+
+ if (par->txbuf.len > 0)
+ kfree(par->txbuf.buf);
fb_deferred_io_cleanup(info);
vfree(info->screen_buffer);
framebuffer_release(info);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-28 4:59 UTC (permalink / raw)
To: andy
Cc: Abdun Nihaal, dan.carpenter, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <cover.1751086324.git.abdun.nihaal@gmail.com>
After commit 56c134f7f1b5 ("fbdev: Track deferred-I/O pages in pageref
struct"), fb_deferred_io_init() allocates memory for info->pagerefs as
well as return an error code on failure. However the error code is
ignored here and the memory allocated could leak because of not calling
fb_deferred_io_cleanup() on the error path.
Fix them by adding the cleanup function on the error path, and handling
the error code returned by fb_deferred_io_init().
Fixes: 56c134f7f1b5 ("fbdev: Track deferred-I/O pages in pageref struct")
Signed-off-by: Abdun Nihaal <abdun.nihaal@gmail.com>
---
v1->v2:
- Handle the error code returned by fb_deferred_io_init correctly
- Update Fixes tag to point to the commit that introduced the memory
allocation which leads to the leak.
drivers/staging/fbtft/fbtft-core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index da9c64152a60..8538b6bab6a5 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -612,7 +612,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
info->fix.line_length = width * bpp / 8;
info->fix.accel = FB_ACCEL_NONE;
info->fix.smem_len = vmem_size;
- fb_deferred_io_init(info);
+ if (fb_deferred_io_init(info))
+ goto release_framebuf;
info->var.rotate = pdata->rotate;
info->var.xres = width;
@@ -652,7 +653,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
if (par->gamma.curves && gamma) {
if (fbtft_gamma_parse_str(par, par->gamma.curves, gamma,
strlen(gamma)))
- goto release_framebuf;
+ goto cleanup_deferred;
}
/* Transmit buffer */
@@ -669,7 +670,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
if (txbuflen > 0) {
txbuf = devm_kzalloc(par->info->device, txbuflen, GFP_KERNEL);
if (!txbuf)
- goto release_framebuf;
+ goto cleanup_deferred;
par->txbuf.buf = txbuf;
par->txbuf.len = txbuflen;
}
@@ -691,6 +692,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
return info;
+cleanup_deferred:
+ fb_deferred_io_cleanup(info);
release_framebuf:
framebuffer_release(info);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] staging: fbtft: cleanup fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-28 4:59 UTC (permalink / raw)
To: andy
Cc: Abdun Nihaal, dan.carpenter, gregkh, lorenzo.stoakes, tzimmermann,
riyandhiman14, willy, notro, thomas.petazzoni, dri-devel,
linux-fbdev, linux-staging, linux-kernel
v2:
- Change the earlier patch to also handle the error code returned by
fb_deferred_io_init() and update Fixes tag to point to the commit that
introduced the memory allocation (which leads to leak).
- Add second patch to make the error handling order symmetric to
fbtft_framebuffer_release() and also remove managed allocation for
txbuf as suggested by Andy and Dan.
Link to v1: https://lore.kernel.org/all/20250626172412.18355-1-abdun.nihaal@gmail.com/
Abdun Nihaal (2):
staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
staging: fbtft: cleanup error handling in fbtft_framebuffer_alloc()
drivers/staging/fbtft/fbtft-core.c | 39 +++++++++++++++++-------------
1 file changed, 22 insertions(+), 17 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: make g_fbmode[] a read-only pointer array
From: Dan Carpenter @ 2025-06-27 18:18 UTC (permalink / raw)
To: Manish Kumar
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <CAAoxDjcYwV9JhRLGJ+opJLEU5j4RHYUt4XZ8E4R9DgF2VqZD8Q@mail.gmail.com>
On Fri, Jun 27, 2025 at 11:16:06PM +0530, Manish Kumar wrote:
> Hi Dan,
>
> Thank you for the review and pointing out the issue. You're right —
> changing the array to `const` makes it unmodifiable, which breaks the
> assignment logic in the driver.
>
> I've reverted the change and will look for another checkpatch warning to
> address for the next version of the patch.
>
Awesome. Thanks.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: make g_fbmode[] a read-only pointer array
From: Manish Kumar @ 2025-06-27 18:15 UTC (permalink / raw)
To: dan.carpenter
Cc: gregkh, linux-fbdev, linux-kernel, linux-staging, manish1588,
sudipm.mukherjee, teddy.wang
In-Reply-To: <c8f5f917-8412-408d-9dd9-6635af8825a7@suswa.mountain>
Hi Dan,
Thank you for the review. You're absolutely right — changing the array to `const` breaks the assignments that follow.
I've reverted this and will send a new cleanup patch later.
Regards,
Manish
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: make g_fbmode[] a read-only pointer array
From: Dan Carpenter @ 2025-06-27 17:35 UTC (permalink / raw)
To: Manish Kumar
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20250627173120.7639-1-manish1588@gmail.com>
On Fri, Jun 27, 2025 at 11:01:20PM +0530, Manish Kumar wrote:
> This fixes a checkpatch warning by changing the declaration of g_fbmode[]
> from 'static const char *' to 'static const char * const', making both the
> string contents and the array elements read-only.
>
> Signed-off-by: Manish Kumar <manish1588@gmail.com>
This breaks the build. Now we can't change the pointer to anythine else
except NULL.
regards,
dan carpenter
^ permalink raw reply
* [PATCH] staging: sm750fb: make g_fbmode[] a read-only pointer array
From: Manish Kumar @ 2025-06-27 17:31 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh
Cc: linux-fbdev, linux-staging, linux-kernel, Manish Kumar
This fixes a checkpatch warning by changing the declaration of g_fbmode[]
from 'static const char *' to 'static const char * const', making both the
string contents and the array elements read-only.
Signed-off-by: Manish Kumar <manish1588@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 1d929aca399c..e77ad73f0db1 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -33,7 +33,7 @@
static int g_hwcursor = 1;
static int g_noaccel;
static int g_nomtrr;
-static const char *g_fbmode[] = {NULL, NULL};
+static const char * const g_fbmode[] = {NULL, NULL};
static const char *g_def_fbmode = "1024x768-32@60";
static char *g_settings;
static int g_dualview;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Abdun Nihaal @ 2025-06-27 16:26 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Dan Carpenter, andy, gregkh, lorenzo.stoakes,
tzimmermann, riyandhiman14, willy, notro, thomas.petazzoni,
dri-devel, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <aF57eMeNafg1w9Qw@smile.fi.intel.com>
Thanks Andy and Dan for your detailed comments. I'll send a V2 with an
another clean up patch that fixes the order and removes devm for txbuf.
Regards,
Nihaal
^ permalink raw reply
* Re: [PATCH v2 5/5] fbdev/simplefb: Add support for interconnect paths
From: Javier Martinez Canillas @ 2025-06-27 11:43 UTC (permalink / raw)
To: Thomas Zimmermann, Luca Weiss, Hans de Goede, Maarten Lankhorst,
Maxime Ripard, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <f7c816a7-e93e-4146-80dc-8fec6113fcea@suse.de>
Thomas Zimmermann <tzimmermann@suse.de> writes:
> Hi
[...]
>> These two functions contain the same logic that you are using in the
>> simpledrm driver. I wonder if could be made helpers so that the code
>> isn't duplicated in both drivers.
>
> No please not!. Any work should rather be directed towards deleting
> simplefb entirely.
>
That is a good point. You are correct that having some duplication to
make easier to get rid of the fbdev driver is a much better approach.
> Best regards
> Thomas
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 5/5] fbdev/simplefb: Add support for interconnect paths
From: Thomas Zimmermann @ 2025-06-27 11:36 UTC (permalink / raw)
To: Javier Martinez Canillas, Luca Weiss, Hans de Goede,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <87ldpdd3dn.fsf@minerva.mail-host-address-is-not-set>
Hi
Am 27.06.25 um 09:56 schrieb Javier Martinez Canillas:
> Luca Weiss <luca.weiss@fairphone.com> writes:
>
>> Some devices might require keeping an interconnect path alive so that
>> the framebuffer continues working. Add support for that by setting the
>> bandwidth requirements appropriately for all provided interconnect
>> paths.
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> ---
>> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 83 insertions(+)
>>
> [...]
>
>> +static void simplefb_detach_icc(void *res)
>> +{
>> + struct simplefb_par *par = res;
>> + int i;
>> +
>> + for (i = par->icc_count - 1; i >= 0; i--) {
>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>> + icc_put(par->icc_paths[i]);
>> + }
>> +}
>> +
>> +static int simplefb_attach_icc(struct simplefb_par *par,
>> + struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + int ret, count, i;
>> +
>> + count = of_count_phandle_with_args(dev->of_node, "interconnects",
>> + "#interconnect-cells");
>> + if (count < 0)
>> + return 0;
>> +
>> + /* An interconnect path consists of two elements */
>> + if (count % 2) {
>> + dev_err(dev, "invalid interconnects value\n");
>> + return -EINVAL;
>> + }
>> + par->icc_count = count / 2;
>> +
>> + par->icc_paths = devm_kcalloc(dev, par->icc_count,
>> + sizeof(*par->icc_paths),
>> + GFP_KERNEL);
>> + if (!par->icc_paths)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < par->icc_count; i++) {
>> + par->icc_paths[i] = of_icc_get_by_index(dev, i);
>> + if (IS_ERR_OR_NULL(par->icc_paths[i])) {
>> + ret = PTR_ERR(par->icc_paths[i]);
>> + if (ret == -EPROBE_DEFER)
>> + goto err;
>> + dev_err(dev, "failed to get interconnect path %u: %d\n", i, ret);
>> + continue;
>> + }
>> +
>> + ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
>> + if (ret) {
>> + dev_err(dev, "failed to set interconnect bandwidth %u: %d\n", i, ret);
>> + continue;
>> + }
>> + }
>> +
>> + return devm_add_action_or_reset(dev, simplefb_detach_icc, par);
>> +
>> +err:
>> + while (i) {
>> + --i;
>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>> + icc_put(par->icc_paths[i]);
>> + }
>> + return ret;
>> +}
>> +#else
> These two functions contain the same logic that you are using in the
> simpledrm driver. I wonder if could be made helpers so that the code
> isn't duplicated in both drivers.
No please not!. Any work should rather be directed towards deleting
simplefb entirely.
Best regards
Thomas
>
> But in any case it could be a follow-up of your series I think.
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Thomas Zimmermann @ 2025-06-27 11:34 UTC (permalink / raw)
To: Krzysztof Kozlowski, Luca Weiss
Cc: Hans de Goede, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Javier Martinez Canillas, Helge Deller, linux-fbdev, dri-devel,
devicetree, linux-kernel
In-Reply-To: <20250627-mysterious-optimistic-bird-acaafb@krzk-bin>
Hi
Am 27.06.25 um 10:08 schrieb Krzysztof Kozlowski:
> On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
>> Document the interconnects property which is a list of interconnect
>> paths that is used by the framebuffer and therefore needs to be kept
>> alive when the framebuffer is being used.
>>
>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> ---
>> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
>> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> @@ -79,6 +79,9 @@ properties:
>> power-domains:
>> description: List of power domains used by the framebuffer.
>>
>> + interconnects:
>> + description: List of interconnect paths used by the framebuffer.
>> +
> maxItems: 1, or this is not a simple FB anymore. Anything which needs
> some sort of resources in unknown way is not simple anymore. You need
> device specific bindings.
In this context, 'simple' means that this device cannot change display
modes or do graphics acceleration. The hardware itself is not
necessarily simple. As Javier pointed out, it's initialized by firmware
on the actual hardware. Think of 'VGA-for-ARM'. We need these resources
to keep the display working.
Best regards
Thomas
>
> Best regards,
> Krzysztof
>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* Re: [PATCH] staging: fbtft: fix potential memory leak in fbtft_framebuffer_alloc()
From: Andy Shevchenko @ 2025-06-27 11:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dan Carpenter, Abdun Nihaal, andy, gregkh, lorenzo.stoakes,
tzimmermann, riyandhiman14, willy, notro, thomas.petazzoni,
dri-devel, linux-fbdev, linux-staging, linux-kernel
In-Reply-To: <aF3CwnHyW5HHzDSG@surfacebook.localdomain>
On Fri, Jun 27, 2025 at 12:59:30AM +0300, Andy Shevchenko wrote:
> Thu, Jun 26, 2025 at 11:11:39PM +0300, Dan Carpenter kirjoitti:
> > On Thu, Jun 26, 2025 at 08:50:43PM +0300, Andy Shevchenko wrote:
> > > On Thu, Jun 26, 2025 at 10:54:10PM +0530, Abdun Nihaal wrote:
...
> Ah, you have a point. Yes, the moving vmem allocation will solve the ordering
> issue.
...with moving from devm for the txbuf. Otherwise we would still have a
problematic ordering with it.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Javier Martinez Canillas @ 2025-06-27 10:06 UTC (permalink / raw)
To: Luca Weiss, Krzysztof Kozlowski
Cc: Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, linux-fbdev,
dri-devel, devicetree, linux-kernel
In-Reply-To: <DAX7ZB27SBPV.2Y0I09TVSF3TT@fairphone.com>
"Luca Weiss" <luca.weiss@fairphone.com> writes:
> Hi Krzysztof,
>
> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote:
>> On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
>>> Document the interconnects property which is a list of interconnect
>>> paths that is used by the framebuffer and therefore needs to be kept
>>> alive when the framebuffer is being used.
>>>
>>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>> ---
>>> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
>>> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>>> @@ -79,6 +79,9 @@ properties:
>>> power-domains:
>>> description: List of power domains used by the framebuffer.
>>>
>>> + interconnects:
>>> + description: List of interconnect paths used by the framebuffer.
>>> +
>>
>> maxItems: 1, or this is not a simple FB anymore. Anything which needs
>> some sort of resources in unknown way is not simple anymore. You need
>> device specific bindings.
>
> The bindings support an arbitrary number of clocks, regulators,
> power-domains. Why should I artificially limit the interconnects to only
> one?
>
I agree with Luca here. There are device specific bindings for the device
specific drivers. But this is about the generic drivers that are able to
scan out using a system provided framebuffer.
The display controller is setup by the firmware but it might need a set
of clocks, power domains, regulators, etc left enabled in order to work.
It's true that the "simple" is a misnomer, probably these drivers should
had been named sysfb and sysfbdrm, or something along those lines.
> The driver code also has that support added in this series.
>
> Regards
> Luca
>
>>
>> Best regards,
>> Krzysztof
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 5/5] fbdev/simplefb: Add support for interconnect paths
From: Javier Martinez Canillas @ 2025-06-27 10:02 UTC (permalink / raw)
To: Luca Weiss, Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <DAX814DZF6AT.31N8TZWL5LMDT@fairphone.com>
"Luca Weiss" <luca.weiss@fairphone.com> writes:
> On Fri Jun 27, 2025 at 9:56 AM CEST, Javier Martinez Canillas wrote:
[...]
>> These two functions contain the same logic that you are using in the
>> simpledrm driver. I wonder if could be made helpers so that the code
>> isn't duplicated in both drivers.
>
> I believe most resource handling code (clocks, regulators,
> power-domains, plus now interconnect) should be pretty generic between
> the two.
>
Yeah.
>>
>> But in any case it could be a follow-up of your series I think.
>
> To be fair, I don't think I'll work on this, I've got plenty of Qualcomm
> SoC-specific bits to work on :)
>
That's OK :) It was just a drive by comment, but as said I don't think
that this code duplication should be a blocker for this patch series.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH v2 5/5] fbdev/simplefb: Add support for interconnect paths
From: Luca Weiss @ 2025-06-27 9:51 UTC (permalink / raw)
To: Javier Martinez Canillas, Hans de Goede, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller
Cc: linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <87ldpdd3dn.fsf@minerva.mail-host-address-is-not-set>
On Fri Jun 27, 2025 at 9:56 AM CEST, Javier Martinez Canillas wrote:
> Luca Weiss <luca.weiss@fairphone.com> writes:
>
>> Some devices might require keeping an interconnect path alive so that
>> the framebuffer continues working. Add support for that by setting the
>> bandwidth requirements appropriately for all provided interconnect
>> paths.
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> ---
>> drivers/video/fbdev/simplefb.c | 83 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 83 insertions(+)
>>
>
> [...]
>
>> +static void simplefb_detach_icc(void *res)
>> +{
>> + struct simplefb_par *par = res;
>> + int i;
>> +
>> + for (i = par->icc_count - 1; i >= 0; i--) {
>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>> + icc_put(par->icc_paths[i]);
>> + }
>> +}
>> +
>> +static int simplefb_attach_icc(struct simplefb_par *par,
>> + struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + int ret, count, i;
>> +
>> + count = of_count_phandle_with_args(dev->of_node, "interconnects",
>> + "#interconnect-cells");
>> + if (count < 0)
>> + return 0;
>> +
>> + /* An interconnect path consists of two elements */
>> + if (count % 2) {
>> + dev_err(dev, "invalid interconnects value\n");
>> + return -EINVAL;
>> + }
>> + par->icc_count = count / 2;
>> +
>> + par->icc_paths = devm_kcalloc(dev, par->icc_count,
>> + sizeof(*par->icc_paths),
>> + GFP_KERNEL);
>> + if (!par->icc_paths)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < par->icc_count; i++) {
>> + par->icc_paths[i] = of_icc_get_by_index(dev, i);
>> + if (IS_ERR_OR_NULL(par->icc_paths[i])) {
>> + ret = PTR_ERR(par->icc_paths[i]);
>> + if (ret == -EPROBE_DEFER)
>> + goto err;
>> + dev_err(dev, "failed to get interconnect path %u: %d\n", i, ret);
>> + continue;
>> + }
>> +
>> + ret = icc_set_bw(par->icc_paths[i], 0, UINT_MAX);
>> + if (ret) {
>> + dev_err(dev, "failed to set interconnect bandwidth %u: %d\n", i, ret);
>> + continue;
>> + }
>> + }
>> +
>> + return devm_add_action_or_reset(dev, simplefb_detach_icc, par);
>> +
>> +err:
>> + while (i) {
>> + --i;
>> + if (!IS_ERR_OR_NULL(par->icc_paths[i]))
>> + icc_put(par->icc_paths[i]);
>> + }
>> + return ret;
>> +}
>> +#else
>
> These two functions contain the same logic that you are using in the
> simpledrm driver. I wonder if could be made helpers so that the code
> isn't duplicated in both drivers.
I believe most resource handling code (clocks, regulators,
power-domains, plus now interconnect) should be pretty generic between
the two.
>
> But in any case it could be a follow-up of your series I think.
To be fair, I don't think I'll work on this, I've got plenty of Qualcomm
SoC-specific bits to work on :)
Regards
Luca
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property
From: Luca Weiss @ 2025-06-27 9:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Hans de Goede, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Javier Martinez Canillas,
Helge Deller, linux-fbdev, dri-devel, devicetree, linux-kernel
In-Reply-To: <20250627-mysterious-optimistic-bird-acaafb@krzk-bin>
Hi Krzysztof,
On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote:
> On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
>> Document the interconnects property which is a list of interconnect
>> paths that is used by the framebuffer and therefore needs to be kept
>> alive when the framebuffer is being used.
>>
>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>> ---
>> Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
>> --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
>> @@ -79,6 +79,9 @@ properties:
>> power-domains:
>> description: List of power domains used by the framebuffer.
>>
>> + interconnects:
>> + description: List of interconnect paths used by the framebuffer.
>> +
>
> maxItems: 1, or this is not a simple FB anymore. Anything which needs
> some sort of resources in unknown way is not simple anymore. You need
> device specific bindings.
The bindings support an arbitrary number of clocks, regulators,
power-domains. Why should I artificially limit the interconnects to only
one?
The driver code also has that support added in this series.
Regards
Luca
>
> Best regards,
> Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox