Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 3/3] video: fbdev: Validate mode timing against monspec
From: David Ung @ 2014-12-03 21:49 UTC (permalink / raw)
  To: linux-fbdev

fbmon may generate mode timings that are out of spec of the monitor.
eg DELL U2410 has a max clock 170mhz but advertises a resolutions of
1920x1200@60 in its Standard Timings, fbmon creates a mode using the
GTF timing calculation which gave it a 193mhz clock.
This patch checks to see if the mode can be supported by the monitor
by comparing against monspecs.dclkmax.

Signed-off-by: David Ung <davidu@nvidia.com>
---
 drivers/video/fbdev/core/fbmon.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index aa1110a..cc3ea6c8 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -496,7 +496,7 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
 }
 
 static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
-		int ver, int rev)
+			  int ver, int rev, struct fb_monspecs *specs)
 {
 	int xres, yres = 0, refresh, ratio, i;
 
@@ -542,16 +542,23 @@ static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
 	if (i = DMT_SIZE || !dmt_modes[i].mode)
 		calc_mode_timings(xres, yres, refresh, mode);
 
+	/* Check the mode we got is within valid spec of the monitor */
+	if (specs && specs->dclkmax
+	    && PICOS2KHZ(mode->pixclock) * 1000 > specs->dclkmax) {
+		DPRINTK("        mode exceed max DCLK\n");
+		return 0;
+	}
+
 	return 1;
 }
 
-static int get_dst_timing(unsigned char *block,
-			  struct fb_videomode *mode, int ver, int rev)
+static int get_dst_timing(unsigned char *block, struct fb_videomode *mode,
+			  int ver, int rev, struct fb_monspecs *specs)
 {
 	int j, num = 0;
 
 	for (j = 0; j < 6; j++, block += STD_TIMING_DESCRIPTION_SIZE)
-		num += get_std_timing(block, &mode[num], ver, rev);
+		num += get_std_timing(block, &mode[num], ver, rev, specs);
 
 	return num;
 }
@@ -607,7 +614,8 @@ static void get_detailed_timing(unsigned char *block,
  * This function builds a mode database using the contents of the EDID
  * data
  */
-static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize)
+static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize,
+					     struct fb_monspecs *specs)
 {
 	struct fb_videomode *mode, *m;
 	unsigned char *block;
@@ -649,12 +657,13 @@ static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize)
 	DPRINTK("   Standard Timings\n");
 	block = edid + STD_TIMING_DESCRIPTIONS_START;
 	for (i = 0; i < STD_TIMING; i++, block += STD_TIMING_DESCRIPTION_SIZE)
-		num += get_std_timing(block, &mode[num], ver, rev);
+		num += get_std_timing(block, &mode[num], ver, rev, specs);
 
 	block = edid + DETAILED_TIMING_DESCRIPTIONS_START;
 	for (i = 0; i < 4; i++, block+= DETAILED_TIMING_DESCRIPTION_SIZE) {
 		if (block[0] = 0x00 && block[1] = 0x00 && block[3] = 0xfa)
-			num += get_dst_timing(block + 5, &mode[num], ver, rev);
+			num += get_dst_timing(block + 5, &mode[num],
+					      ver, rev, specs);
 	}
 
 	/* Yikes, EDID data is totally useless */
@@ -713,7 +722,7 @@ static int fb_get_monitor_limits(unsigned char *edid, struct fb_monspecs *specs)
 		int num_modes, hz, hscan, pixclock;
 		int vtotal, htotal;
 
-		modes = fb_create_modedb(edid, &num_modes);
+		modes = fb_create_modedb(edid, &num_modes, specs);
 		if (!modes) {
 			DPRINTK("None Available\n");
 			return 1;
@@ -970,7 +979,7 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 	DPRINTK("   Display Characteristics:\n");
 	get_monspecs(edid, specs);
 
-	specs->modedb = fb_create_modedb(edid, &specs->modedb_len);
+	specs->modedb = fb_create_modedb(edid, &specs->modedb_len, specs);
 
 	/*
 	 * Workaround for buggy EDIDs that sets that the first
-- 
1.8.1.5


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply related

* RE: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Li.Xiubo @ 2014-12-04  1:56 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: plagnioj@jcrosoft.com, tomi.valkeinen@ti.com,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	shawn.guo@linaro.org, alexander.stein@systec-electronic.com
In-Reply-To: <4810522.t9QFQYHtHI@wuerfel>

Hi Arnd,

Thanks for you advice.

Because this is very emergency for customers and have delayed for monthes, and the
DRM/KMS will have a little long time to be supported. So I'd like to add this first
and will add DRM/KMS version later.

Thanks very much,

BRs
Xiubo


> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Arnd Bergmann
> Sent: Wednesday, December 03, 2014 5:52 PM
> To: linux-arm-kernel@lists.infradead.org
> Cc: Xiubo Li-B47053; plagnioj@jcrosoft.com; tomi.valkeinen@ti.com; linux-
> fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; shawn.guo@linaro.org;
> alexander.stein@systec-electronic.com
> Subject: Re: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
> 
> On Wednesday 03 December 2014 17:15:58 Xiubo Li wrote:
> > Framebuffer driver for the Freescale SoC Display Controller.
> >
> >
> > Change in V2:
> > - Use the native-mode timing as default.
> > - Add to_fsl_private().
> 
> <standard reply>
> 
> We try to not have any new framebuffer drivers. Please consider making
> this a DRM/KMS driver instead.
> 
> </standard reply>
> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v10 2/6] backlight: Add support Skyworks SKY81452 backlight driver
From: Gyungoh Yoo @ 2014-12-04  2:04 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: sameo-VuQAYsv1563Yd54FQh9/CA, lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jg1.han-Sze3O3UU22JBDgjK7y7TUQ,
	cooloney-Re5JQEeQqe8AvxtiuMwx3w, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	jic23-DgEjT+Ai2ygdnm+yROfE0A,
	thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	ktsai-GubuWUlQtMwciDkP5Hr2oA, hs-ynQEQJNshbs,
	stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ,
	matti.vaittinen-OYasijW0DpE, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	jason-NLaQJdtUoK4Be96aLqz0jA, heiko-4mtYJXux2i+zQB+pC5nmwQ,
	shawn.guo-KZfg59tc24xl57MIdRCFDg, florian.vaussard-p8DiymsW2f8,
	andrew-g2DYL2Zd6BY, antonynpavlov-Re5JQEeQqe8AvxtiuMwx3w,
	hytszk-Re5JQEeQqe8AvxtiuMwx3w, plagnioj-sclMFOaUSTBWk0Htik3J/w,
	tomi.valkeinen-l0cyMroinI0, jack.yoo-tjhQNA90jdKqndwCJWfcng,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1417600949.12511.7.camel-AfvqVibwNMkMNNZnWhT/Jw@public.gmane.org>

On Wed, Dec 03, 2014 at 11:02:29AM +0100, Oliver Neukum wrote:
> On Wed, 2014-12-03 at 16:05 +0900, gyungoh@gmail.com wrote:
> > +static ssize_t sky81452_bl_store_enable(struct device *dev,
> > +               struct device_attribute *attr, const char *buf, size_t
> > count)
> > +{
> > +       struct regmap *regmap = bl_get_data(to_backlight_device(dev));
> > +       unsigned long value;
> > +       int ret;
> > +
> > +       ret = kstrtoul(buf, 16, &value);
> > +       if (IS_ERR_VALUE(ret))
> > +               return ret;
> > +
> > +       ret = regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
> > +                                       value << CTZ(SKY81452_EN));
> 
> No range checking for value?

Avaiable range is from 0 to SKY81452_EN.
regmap_update_bits() is masking the value.

> 
> > +       if (IS_ERR_VALUE(ret))
> > +               return ret;
> > +
> > +       return count;
> > +}
> 
> 	Regards
> 		Oliver
> 
> -- 
> Oliver Neukum <oneukum@suse.de>
> 

^ permalink raw reply

* Re: [PATCH 1/3] video: fbdev: vt8623fb: suppress build warning
From: Tomi Valkeinen @ 2014-12-04  7:05 UTC (permalink / raw)
  To: Prabhakar Lad; +Cc: Jean-Christophe Plagniol-Villard, LFBDEV, LKML
In-Reply-To: <CA+V-a8vnv4B77XvbzwNkJk=vqGCFWoXxFoyUf0ZNiKwa+gY2ug@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

On 03/12/14 20:29, Prabhakar Lad wrote:
> On Wed, Dec 3, 2014 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 27/11/14 00:07, Lad, Prabhakar wrote:
>>> this patch fixes following build warning:
>>> drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
>>> drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>>                        ^
>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>> ---
>>>  drivers/video/fbdev/vt8623fb.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
>>> index 5c7cbc6..ea7f056 100644
>>> --- a/drivers/video/fbdev/vt8623fb.c
>>> +++ b/drivers/video/fbdev/vt8623fb.c
>>> @@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>>
>>>       pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
>>>
>>> -     par->state.vgabase = (void __iomem *) vga_res.start;
>>> +     par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
>>
>> This does look quite ugly... Where does the warning come from in the
>> first place. Isn't vga_res.start (resource_size_t) the size of a pointer?
>>
> Yes looks ugly, I am not sure what you meant from 'where does this warning
> come from' its in the commit message.

I meant why is there a warning at all. With a quick glance,
vga_res.start is the size of a pointer. So the sizes of the integer and
the pointer should be the same. But the warning still says "of different
size".

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] video: fbdev: vt8623fb: suppress build warning
From: Tomi Valkeinen @ 2014-12-04  7:56 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: Sudip Mukherjee, Jean-Christophe Plagniol-Villard, LFBDEV, LKML
In-Reply-To: <20141204074641.GB4311@sudip-PC>

[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]

On 04/12/14 09:46, Sudip Mukherjee wrote:
> On Thu, Dec 04, 2014 at 09:05:46AM +0200, Tomi Valkeinen wrote:
>> On 03/12/14 20:29, Prabhakar Lad wrote:
>>> On Wed, Dec 3, 2014 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>> On 27/11/14 00:07, Lad, Prabhakar wrote:
>>>>> this patch fixes following build warning:
>>>>> drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
>>>>> drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>>>>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>>>>                        ^
>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>>> ---
>>>>>  drivers/video/fbdev/vt8623fb.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
>>>>> index 5c7cbc6..ea7f056 100644
>>>>> --- a/drivers/video/fbdev/vt8623fb.c
>>>>> +++ b/drivers/video/fbdev/vt8623fb.c
>>>>> @@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>>>>
>>>>>       pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
>>>>>
>>>>> -     par->state.vgabase = (void __iomem *) vga_res.start;
>>>>> +     par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
>>>>
>>>> This does look quite ugly... Where does the warning come from in the
>>>> first place. Isn't vga_res.start (resource_size_t) the size of a pointer?
>>>>
>>> Yes looks ugly, I am not sure what you meant from 'where does this warning
>>> come from' its in the commit message.
>>
>> I meant why is there a warning at all. With a quick glance,
>> vga_res.start is the size of a pointer. So the sizes of the integer and
>> the pointer should be the same. But the warning still says "of different
>> size".
> 
> poking my nose into your discussion.
> I tried to see the warning, and I re-compiled like make W=1 M=drivers/video/fbdev/ (before that make clean M=drivers/video/fbdev was done)
> I can see warning with many other files, but drivers/video/fbdev/vt8623fb.o was quiet and there was no warning.
> I tested with next=20141203.
> did i miss something in checking the warning ?

I don't see the warning either when compiling for arm or x86_64. On what
architecture do you see the warning?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] video: fbdev: vt8623fb: suppress build warning
From: Sudip Mukherjee @ 2014-12-04  7:58 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Prabhakar Lad, Jean-Christophe Plagniol-Villard, LFBDEV, LKML
In-Reply-To: <548007CA.7010900@ti.com>

On Thu, Dec 04, 2014 at 09:05:46AM +0200, Tomi Valkeinen wrote:
> On 03/12/14 20:29, Prabhakar Lad wrote:
> > On Wed, Dec 3, 2014 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> On 27/11/14 00:07, Lad, Prabhakar wrote:
> >>> this patch fixes following build warning:
> >>> drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
> >>> drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >>>   par->state.vgabase = (void __iomem *) vga_res.start;
> >>>                        ^
> >>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> >>> ---
> >>>  drivers/video/fbdev/vt8623fb.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
> >>> index 5c7cbc6..ea7f056 100644
> >>> --- a/drivers/video/fbdev/vt8623fb.c
> >>> +++ b/drivers/video/fbdev/vt8623fb.c
> >>> @@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >>>
> >>>       pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
> >>>
> >>> -     par->state.vgabase = (void __iomem *) vga_res.start;
> >>> +     par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
> >>
> >> This does look quite ugly... Where does the warning come from in the
> >> first place. Isn't vga_res.start (resource_size_t) the size of a pointer?
> >>
> > Yes looks ugly, I am not sure what you meant from 'where does this warning
> > come from' its in the commit message.
> 
> I meant why is there a warning at all. With a quick glance,
> vga_res.start is the size of a pointer. So the sizes of the integer and
> the pointer should be the same. But the warning still says "of different
> size".

poking my nose into your discussion.
I tried to see the warning, and I re-compiled like make W=1 M=drivers/video/fbdev/ (before that make clean M=drivers/video/fbdev was done)
I can see warning with many other files, but drivers/video/fbdev/vt8623fb.o was quiet and there was no warning.
I tested with next 141203.
did i miss something in checking the warning ?

thanks
sudip

> 
>  Tomi
> 
> 



^ permalink raw reply

* Re: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Tomi Valkeinen @ 2014-12-04  8:10 UTC (permalink / raw)
  To: Li.Xiubo@freescale.com, Arnd Bergmann
  Cc: plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, shawn.guo@linaro.org,
	alexander.stein@systec-electronic.com
In-Reply-To: <BY2PR0301MB06131C2C901351E8592ED3D89B780@BY2PR0301MB0613.namprd03.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

Hi Xiubo,

On 04/12/14 03:56, Li.Xiubo@freescale.com wrote:
> Hi Arnd,
> 
> Thanks for you advice.
> 
> Because this is very emergency for customers and have delayed for monthes, and the
> DRM/KMS will have a little long time to be supported. So I'd like to add this first
> and will add DRM/KMS version later.

I would also suggest to just write a drm driver for this. I don't see
anything special in this driver that would be better supported via
fbdev. With drm you'll get a more modern framework, and you will still
have the same /dev/fbX device to use for legacy userspace software.

What do you mean with "delayed for months"? The first version of this
series (that I can find) was posted less than two weeks ago.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* RE: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Li.Xiubo @ 2014-12-04  8:30 UTC (permalink / raw)
  To: Tomi Valkeinen, Arnd Bergmann
  Cc: plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, shawn.guo@linaro.org,
	alexander.stein@systec-electronic.com
In-Reply-To: <548016D9.6030006@ti.com>

Hi Tomi,



> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@ti.com]
> Sent: Thursday, December 04, 2014 4:10 PM
> To: Xiubo Li-B47053; Arnd Bergmann
> Cc: plagnioj@jcrosoft.com; linux-fbdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; shawn.guo@linaro.org; alexander.stein@systec-
> electronic.com
> Subject: Re: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
> 
> Hi Xiubo,
> 
> On 04/12/14 03:56, Li.Xiubo@freescale.com wrote:
> > Hi Arnd,
> >
> > Thanks for you advice.
> >
> > Because this is very emergency for customers and have delayed for monthes,
> and the
> > DRM/KMS will have a little long time to be supported. So I'd like to add
> this first
> > and will add DRM/KMS version later.
> 
> I would also suggest to just write a drm driver for this. I don't see
> anything special in this driver that would be better supported via
> fbdev. With drm you'll get a more modern framework, and you will still
> have the same /dev/fbX device to use for legacy userspace software.
> 
> What do you mean with "delayed for months"? The first version of this
> series (that I can find) was posted less than two weeks ago.
> 

Sorry for confusing, it was delayed for other reasons internal.

I am not familiar about the DRM, and I'd like to know if the DRM driver will be
support, should I also develop the libdrm too ? Or just coding in kernel level ?
Is there any Document about how to have /dev/fbX device to use ?

If possible, I'd like this could be accept for this time. And I will add the DRM
Version later(for developing and testing will take a long time).

Thanks very much,

BRs
Xiubo


>  Tomi
> 


^ permalink raw reply

* Re: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Tomi Valkeinen @ 2014-12-04  9:09 UTC (permalink / raw)
  To: Li.Xiubo@freescale.com
  Cc: Arnd Bergmann, plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, shawn.guo@linaro.org,
	alexander.stein@systec-electronic.com
In-Reply-To: <BY2PR0301MB0613ABEA1410C04CF07C1C7C9B780@BY2PR0301MB0613.namprd03.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On 04/12/14 10:30, Li.Xiubo@freescale.com wrote:

> Sorry for confusing, it was delayed for other reasons internal.
> 
> I am not familiar about the DRM, and I'd like to know if the DRM driver will be
> support, should I also develop the libdrm too ? Or just coding in kernel level ?

For simple drm drivers (this looks like it would be a simple one), I
don't think there's any need for libdrm support. The generic DRM
interfaces should be enough, so just kernel level coding needed.

> Is there any Document about how to have /dev/fbX device to use ?

There's DRM documentation here:

https://www.kernel.org/doc/htmldocs/drm/index.html

And many existing drivers to use as examples. The dri-devel list
(http://lists.freedesktop.org/mailman/listinfo/dri-devel), which is the
mailing list used for DRM development, is active and you probably can
get more support from there than from the fbdev list.

It should not be a huge effort to write a drm driver for a simple LCD
controller like this. I would bet that you can write a working driver in
a week.

> If possible, I'd like this could be accept for this time. And I will add the DRM
> Version later(for developing and testing will take a long time).

I'm sorry but "it was delayed for internal reasons" and "our customer
needs this driver" are not very good reasons for getting a driver merged
to mainline Linux.

You can provide your driver to your customer as a separate patch series
which they can apply. There should be no conflicts or other issues
there, so it should be simple.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCHv2 1/4] video: fsl-dcfb: Add dcfb framebuffer driver for LS1021A platform
From: Alexander Stein @ 2014-12-04  9:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1417598162-40433-2-git-send-email-Li.Xiubo@freescale.com>

On Wednesday 03 December 2014 17:15:59, Xiubo Li wrote:
> +	tnp = of_find_node_by_name(dnp, "display-timings");
> +	if (!tnp) {
> +		dev_err(dcfb->dev, "failed to find \"display-timings\" node\n");
> +		return -ENODEV;
> +		goto put_dnp;
> +	}
> +
> +	for (i = 0; i < of_get_child_count(tnp); i++) {
> +		struct videomode vm;
> +
> +		ret = videomode_from_timings(timings, &vm, i);
> +		if (ret < 0)
> +			goto put_tnp;
> +
> +		ret = fb_videomode_from_videomode(&vm, &fb_vm);
> +		if (ret < 0)
> +			goto put_tnp;
> +
> +		fb_add_videomode(&fb_vm, &info->modelist);
> +	}
> +
> +	ret = of_get_fb_videomode(dnp, &fb_vm, OF_USE_NATIVE_MODE);
> +	if (ret)
> +		goto put_dnp;

Souldn't this be put_tnp like in the loop above? 

> +	fb_videomode_to_var(&info->var, &fb_vm);
> +	ret = fsl_dcfb_check_var(&info->var, info);

But picking the native mode per default looks nice to be! :)

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082


^ permalink raw reply

* RE: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Li.Xiubo @ 2014-12-04  9:54 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Arnd Bergmann, plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, shawn.guo@linaro.org,
	alexander.stein@systec-electronic.com
In-Reply-To: <548024C7.3060004@ti.com>

Hi Tomi,

Thanks for your information very much.

> > Sorry for confusing, it was delayed for other reasons internal.
> >
> > I am not familiar about the DRM, and I'd like to know if the DRM driver will
> be
> > support, should I also develop the libdrm too ? Or just coding in kernel
> level ?
> 
> For simple drm drivers (this looks like it would be a simple one), I
> don't think there's any need for libdrm support. The generic DRM
> interfaces should be enough, so just kernel level coding needed.
> 

That's to say, if I am using the X11 server without any code in usrspace,
If the DRM driver will support /dev/fbX, it could work correctly ?

Thanks,

BRs
Xiubo





^ permalink raw reply

* Re: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Tomi Valkeinen @ 2014-12-04  9:59 UTC (permalink / raw)
  To: Li.Xiubo@freescale.com
  Cc: Arnd Bergmann, plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, shawn.guo@linaro.org,
	alexander.stein@systec-electronic.com
In-Reply-To: <BY2PR0301MB061372E00518F169BCF2FFFA9B780@BY2PR0301MB0613.namprd03.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

On 04/12/14 11:54, Li.Xiubo@freescale.com wrote:
> Hi Tomi,
> 
> Thanks for your information very much.
> 
>>> Sorry for confusing, it was delayed for other reasons internal.
>>>
>>> I am not familiar about the DRM, and I'd like to know if the DRM driver will
>> be
>>> support, should I also develop the libdrm too ? Or just coding in kernel
>> level ?
>>
>> For simple drm drivers (this looks like it would be a simple one), I
>> don't think there's any need for libdrm support. The generic DRM
>> interfaces should be enough, so just kernel level coding needed.
>>
> 
> That's to say, if I am using the X11 server without any code in usrspace,
> If the DRM driver will support /dev/fbX, it could work correctly ?

Yes. DRM offers helper code to add /dev/fbX with not too many lines. You
can look at the docs and drm_fb_helper.c.

And X11 should work fine on top of that, using the X11 fbdev support.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* RE: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Li.Xiubo @ 2014-12-04 10:03 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Arnd Bergmann, plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, shawn.guo@linaro.org,
	alexander.stein@systec-electronic.com
In-Reply-To: <5480308F.6070607@ti.com>

Hi Tomi,

Thanks very much for your help.

I will have a try these days.

BRs
Xiubo


> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@ti.com]
> Sent: Thursday, December 04, 2014 6:00 PM
> To: Xiubo Li-B47053
> Cc: Arnd Bergmann; plagnioj@jcrosoft.com; linux-fbdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; shawn.guo@linaro.org; alexander.stein@systec-
> electronic.com
> Subject: Re: [PATCHv2 0/4] LS1021A: Add dcfb framebuffer driver support.
> 
> On 04/12/14 11:54, Li.Xiubo@freescale.com wrote:
> > Hi Tomi,
> >
> > Thanks for your information very much.
> >
> >>> Sorry for confusing, it was delayed for other reasons internal.
> >>>
> >>> I am not familiar about the DRM, and I'd like to know if the DRM driver
> will
> >> be
> >>> support, should I also develop the libdrm too ? Or just coding in kernel
> >> level ?
> >>
> >> For simple drm drivers (this looks like it would be a simple one), I
> >> don't think there's any need for libdrm support. The generic DRM
> >> interfaces should be enough, so just kernel level coding needed.
> >>
> >
> > That's to say, if I am using the X11 server without any code in usrspace,
> > If the DRM driver will support /dev/fbX, it could work correctly ?
> 
> Yes. DRM offers helper code to add /dev/fbX with not too many lines. You
> can look at the docs and drm_fb_helper.c.
> 
> And X11 should work fine on top of that, using the X11 fbdev support.
> 
>  Tomi
> 


^ permalink raw reply

* Re: [PATCH 1/3] video: fbdev: vt8623fb: suppress build warning
From: Geert Uytterhoeven @ 2014-12-04 13:29 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Prabhakar Lad, Sudip Mukherjee, Jean-Christophe Plagniol-Villard,
	LFBDEV, LKML
In-Reply-To: <548013B0.4080108@ti.com>

On Thu, Dec 4, 2014 at 8:56 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 04/12/14 09:46, Sudip Mukherjee wrote:
>> On Thu, Dec 04, 2014 at 09:05:46AM +0200, Tomi Valkeinen wrote:
>>> On 03/12/14 20:29, Prabhakar Lad wrote:
>>>> On Wed, Dec 3, 2014 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>>> On 27/11/14 00:07, Lad, Prabhakar wrote:
>>>>>> this patch fixes following build warning:
>>>>>> drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
>>>>>> drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>>>>>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>>>>>                        ^
>>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>>>> ---
>>>>>>  drivers/video/fbdev/vt8623fb.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
>>>>>> index 5c7cbc6..ea7f056 100644
>>>>>> --- a/drivers/video/fbdev/vt8623fb.c
>>>>>> +++ b/drivers/video/fbdev/vt8623fb.c
>>>>>> @@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>>>>>
>>>>>>       pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
>>>>>>
>>>>>> -     par->state.vgabase = (void __iomem *) vga_res.start;
>>>>>> +     par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
>>>>>
>>>>> This does look quite ugly... Where does the warning come from in the
>>>>> first place. Isn't vga_res.start (resource_size_t) the size of a pointer?
>>>>>
>>>> Yes looks ugly, I am not sure what you meant from 'where does this warning
>>>> come from' its in the commit message.
>>>
>>> I meant why is there a warning at all. With a quick glance,
>>> vga_res.start is the size of a pointer. So the sizes of the integer and
>>> the pointer should be the same. But the warning still says "of different
>>> size".
>>
>> poking my nose into your discussion.
>> I tried to see the warning, and I re-compiled like make W=1 M=drivers/video/fbdev/ (before that make clean M=drivers/video/fbdev was done)
>> I can see warning with many other files, but drivers/video/fbdev/vt8623fb.o was quiet and there was no warning.
>> I tested with next 141203.
>> did i miss something in checking the warning ?
>
> I don't see the warning either when compiling for arm or x86_64. On what
> architecture do you see the warning?

On 32-bit systems with PHYS_ADDR_T_64BIT=y, resource_size_t is u64,
while pointers are still 32-bit.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 1/3] video: fbdev: vt8623fb: suppress build warning
From: Tomi Valkeinen @ 2014-12-04 13:40 UTC (permalink / raw)
  To: Geert Uytterhoeven, Prabhakar Lad
  Cc: Sudip Mukherjee, Jean-Christophe Plagniol-Villard, LFBDEV, LKML
In-Reply-To: <CAMuHMdX6rbaPYFpSR7nGf6Di0G=cGL9yFZLLpjUDi7TLyu1wmw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2864 bytes --]

On 04/12/14 15:29, Geert Uytterhoeven wrote:
> On Thu, Dec 4, 2014 at 8:56 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 04/12/14 09:46, Sudip Mukherjee wrote:
>>> On Thu, Dec 04, 2014 at 09:05:46AM +0200, Tomi Valkeinen wrote:
>>>> On 03/12/14 20:29, Prabhakar Lad wrote:
>>>>> On Wed, Dec 3, 2014 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>>>> On 27/11/14 00:07, Lad, Prabhakar wrote:
>>>>>>> this patch fixes following build warning:
>>>>>>> drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
>>>>>>> drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>>>>>>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>>>>>>                        ^
>>>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>>>>> ---
>>>>>>>  drivers/video/fbdev/vt8623fb.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
>>>>>>> index 5c7cbc6..ea7f056 100644
>>>>>>> --- a/drivers/video/fbdev/vt8623fb.c
>>>>>>> +++ b/drivers/video/fbdev/vt8623fb.c
>>>>>>> @@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>>>>>>
>>>>>>>       pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
>>>>>>>
>>>>>>> -     par->state.vgabase = (void __iomem *) vga_res.start;
>>>>>>> +     par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
>>>>>>
>>>>>> This does look quite ugly... Where does the warning come from in the
>>>>>> first place. Isn't vga_res.start (resource_size_t) the size of a pointer?
>>>>>>
>>>>> Yes looks ugly, I am not sure what you meant from 'where does this warning
>>>>> come from' its in the commit message.
>>>>
>>>> I meant why is there a warning at all. With a quick glance,
>>>> vga_res.start is the size of a pointer. So the sizes of the integer and
>>>> the pointer should be the same. But the warning still says "of different
>>>> size".
>>>
>>> poking my nose into your discussion.
>>> I tried to see the warning, and I re-compiled like make W=1 M=drivers/video/fbdev/ (before that make clean M=drivers/video/fbdev was done)
>>> I can see warning with many other files, but drivers/video/fbdev/vt8623fb.o was quiet and there was no warning.
>>> I tested with next=20141203.
>>> did i miss something in checking the warning ?
>>
>> I don't see the warning either when compiling for arm or x86_64. On what
>> architecture do you see the warning?
> 
> On 32-bit systems with PHYS_ADDR_T_64BIT=y, resource_size_t is u64,
> while pointers are still 32-bit.

Ah, I see. Yes, I can reproduce the warning with that config.

So, still rather ugly, but looks correct to me, so I'll apply the series.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 1/1] video: fbdev-LCDC: Deletion of an unnecessary check before the function call "vfree"
From: Tomi Valkeinen @ 2014-12-04 14:19 UTC (permalink / raw)
  To: SF Markus Elfring, Jean-Christophe Plagniol-Villard, linux-fbdev
  Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5470B315.8000507@users.sourceforge.net>

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

Hi,

On 22/11/14 18:00, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 22 Nov 2014 16:51:31 +0100
> 
> The vfree() function performs also input parameter validation.
> Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/video/fbdev/sh_mobile_lcdcfb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> index 2bcc84a..cfde21d 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -2181,8 +2181,7 @@ sh_mobile_lcdc_channel_fb_cleanup(struct sh_mobile_lcdc_chan *ch)
>  	if (!info || !info->device)
>  		return;
>  
> -	if (ch->sglist)
> -		vfree(ch->sglist);
> +	vfree(ch->sglist);
>  
>  	fb_dealloc_cmap(&info->cmap);
>  	framebuffer_release(info);

Thanks, I've applied the fbdev patches. Next time, please use
git-format-patch and git-send-email to send a proper patch series.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] arm: boot: dts: am437x-sk: fix lcd enable pin mux data
From: Felipe Balbi @ 2014-12-04 14:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <543E6774.8000406@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

HI,

On Wed, Oct 15, 2014 at 03:24:20PM +0300, Tomi Valkeinen wrote:
> On 14/10/14 21:28, Felipe Balbi wrote:
> > Caused by a copy & paste error. Note that even with
> > this bug AM437x SK display still works because GPIO
> > mux mode is always enabled. It's still wrong to mux
> > somebody else's pin.
> > 
> > Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
> > isn't used for anything.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
> > index 859ff3d..681be00 100644
> > --- a/arch/arm/boot/dts/am437x-sk-evm.dts
> > +++ b/arch/arm/boot/dts/am437x-sk-evm.dts
> > @@ -320,8 +320,7 @@
> >  
> >  	lcd_pins: lcd_pins {
> >  		pinctrl-single,pins = <
> > -			/* GPIO 5_8 to select LCD / HDMI */
> > -			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
> > +			0x1c (PIN_OUTPUT_PULLUP | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
> >  		>;
> >  	};
> >  };
> 
> I didn't verify the offset, but based on the comments this looks fine to me.
> 
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Tony, are you taking this patch ? Looks like it has been forgotten and
now it needs to be backported to v3.17 and v3.18 (assuming it's too late
for v3.18).

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] video: fbdev: Add additional vesa modes
From: Tomi Valkeinen @ 2014-12-04 14:53 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417643369-20603-1-git-send-email-davidu@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]

On 03/12/14 23:49, David Ung wrote:
> Add high resolution modes to vesa_modes struct.
> 
> Signed-off-by: David Ung <davidu@nvidia.com>
> ---
>  drivers/video/fbdev/core/modedb.c | 27 +++++++++++++++++++++++++++
>  include/linux/fb.h                |  2 +-
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
> index 388f797..0b57c1df 100644
> --- a/drivers/video/fbdev/core/modedb.c
> +++ b/drivers/video/fbdev/core/modedb.c
> @@ -468,6 +468,33 @@ const struct fb_videomode vesa_modes[] = {
>  	/* 33 1920x1440-75 VESA */
>  	{ NULL, 75, 1920, 1440, 3367, 352, 144, 56, 1, 224, 3,
>  	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 34 1920x1200-60 RB VESA */
> +	{ NULL, 60, 1920, 1200, 6493, 80, 48, 26, 3, 32, 6,
> +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 35 1920x1200-60 VESA */
> +	{ NULL, 60, 1920, 1200, 5174, 336, 136, 36, 3, 200, 6,
> +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 36 1920x1200-75 VESA */
> +	{ NULL, 75, 1920, 1200, 4077, 344, 136, 46, 3, 208, 6,
> +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 37 1920x1200-85 VESA */
> +	{ NULL, 85, 1920, 1200, 3555, 352, 144, 53, 3, 208, 6,
> +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 38 2560x1600-60 RB VESA */
> +	{ NULL, 60, 2560, 1600, 3724, 80, 48, 37, 3, 32, 6,
> +	  FB_SYNC_HOR_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 39 2560x1600-60 VESA */
> +	{ NULL, 60, 2560, 1600, 2869, 472, 192, 49, 3, 280, 6,
> +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 40 2560x1600-75 VESA */
> +	{ NULL, 75, 2560, 1600, 2256, 488, 208, 63, 3, 280, 6,
> +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 41 2560x1600-85 VESA */
> +	{ NULL, 85, 2560, 1600, 1979, 488, 208, 73, 3, 280, 6,
> +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
> +	/* 42 2560x1600-120 RB VESA */
> +	{ NULL, 120, 2560, 1600, 1809, 80, 48, 85, 3, 32, 6,
> +	  FB_SYNC_HOR_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },

Where did you take these timings? Are the modes in vesa_modes[] in some
defined order, or just in the order they have been added?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] arm: boot: dts: am437x-sk: fix lcd enable pin mux data
From: Felipe Balbi @ 2014-12-04 15:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20141204144325.GA17269@saruman>

[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]

On Thu, Dec 04, 2014 at 08:43:25AM -0600, Felipe Balbi wrote:
> HI,
> 
> On Wed, Oct 15, 2014 at 03:24:20PM +0300, Tomi Valkeinen wrote:
> > On 14/10/14 21:28, Felipe Balbi wrote:
> > > Caused by a copy & paste error. Note that even with
> > > this bug AM437x SK display still works because GPIO
> > > mux mode is always enabled. It's still wrong to mux
> > > somebody else's pin.
> > > 
> > > Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
> > > isn't used for anything.
> > > 
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > ---
> > >  arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
> > > index 859ff3d..681be00 100644
> > > --- a/arch/arm/boot/dts/am437x-sk-evm.dts
> > > +++ b/arch/arm/boot/dts/am437x-sk-evm.dts
> > > @@ -320,8 +320,7 @@
> > >  
> > >  	lcd_pins: lcd_pins {
> > >  		pinctrl-single,pins = <
> > > -			/* GPIO 5_8 to select LCD / HDMI */
> > > -			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
> > > +			0x1c (PIN_OUTPUT_PULLUP | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
> > >  		>;
> > >  	};
> > >  };
> > 
> > I didn't verify the offset, but based on the comments this looks fine to me.
> > 
> > Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Tony, are you taking this patch ? Looks like it has been forgotten and
> now it needs to be backported to v3.17 and v3.18 (assuming it's too late
> for v3.18).

I'll send a new version and Cc stable.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] video: fbdev: Check Standard Timing against DMT
From: Tomi Valkeinen @ 2014-12-04 15:41 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417643369-20603-2-git-send-email-davidu@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 3006 bytes --]

On 03/12/14 23:49, David Ung wrote:
> Add the VESA Display Monitor Timing (DMT) table.
> During parsing of Standard Timings, it compare the 2 byte STD code
> with DMT to see what the VESA mode should be.  If there is no entry
> in the vesa_modes table or no match found, it fallsback to the
> GTF timings.
> 
> Signed-off-by: David Ung <davidu@nvidia.com>
> ---
>  drivers/video/fbdev/core/fbmon.c  | 20 ++++++----
>  drivers/video/fbdev/core/modedb.c | 84 +++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h                | 10 +++++
>  3 files changed, 107 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
> index 5b0e313..aa1110a 100644
> --- a/drivers/video/fbdev/core/fbmon.c
> +++ b/drivers/video/fbdev/core/fbmon.c
> @@ -526,16 +526,22 @@ static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
>  	refresh = (block[1] & 0x3f) + 60;
>  
>  	DPRINTK("      %dx%d@%dHz\n", xres, yres, refresh);
> -	for (i = 0; i < VESA_MODEDB_SIZE; i++) {
> -		if (vesa_modes[i].xres == xres &&
> -		    vesa_modes[i].yres == yres &&
> -		    vesa_modes[i].refresh == refresh) {
> -			*mode = vesa_modes[i];
> +	for (i = 0; i < DMT_SIZE; i++) {
> +		u32 std_2byte_code = block[0] << 8 | block[1];
> +
> +		if (std_2byte_code == dmt_modes[i].std_2byte_code) {
> +			if (!dmt_modes[i].mode)
> +				break;
> +			*mode = *dmt_modes[i].mode;
>  			mode->flag |= FB_MODE_IS_STANDARD;
> -			return 1;
> +			DPRINTK("        DMT id=%d\n", dmt_modes[i].dmt_id);
> +			break;
>  		}
>  	}
> -	calc_mode_timings(xres, yres, refresh, mode);
> +
> +	if (i == DMT_SIZE || !dmt_modes[i].mode)
> +		calc_mode_timings(xres, yres, refresh, mode);
> +
>  	return 1;
>  }
>  
> diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
> index 0b57c1df..858a97e 100644
> --- a/drivers/video/fbdev/core/modedb.c
> +++ b/drivers/video/fbdev/core/modedb.c
> @@ -497,6 +497,90 @@ const struct fb_videomode vesa_modes[] = {
>  	  FB_SYNC_HOR_HIGH_ACT, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
>  };
>  EXPORT_SYMBOL(vesa_modes);
> +
> +const struct dmt_videomode dmt_modes[DMT_SIZE] = {
> +	{ 0x01, 0x0000, 0x000000, &vesa_modes[0] },
> +	{ 0x02, 0x3119, 0x000000, &vesa_modes[1] },
> +	{ 0x03, 0x0000, 0x000000, &vesa_modes[2] },
> +	{ 0x04, 0x3140, 0x000000, &vesa_modes[3] },
> +	{ 0x05, 0x314c, 0x000000, &vesa_modes[4] },
> +	{ 0x06, 0x314f, 0x000000, &vesa_modes[5] },
> +	{ 0x07, 0x3159, 0x000000, &vesa_modes[6] },
> +	{ 0x08, 0x0000, 0x000000, &vesa_modes[7] },
> +	{ 0x09, 0x4540, 0x000000, &vesa_modes[8] },
> +	{ 0x0a, 0x454c, 0x000000, &vesa_modes[9] },
> +	{ 0x0b, 0x454f, 0x000000, &vesa_modes[10] },
> +	{ 0x0c, 0x4559, 0x000000, &vesa_modes[11] },
> +	{ 0x0d, 0x0000, 0x000000, 0 },
> +	{ 0x0e, 0x0000, 0x000000, 0 },

You've filled only some of the modes in this table. What's the logic
which modes are left out?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* RE: [PATCH 1/3] video: fbdev: Add additional vesa modes
From: David Ung @ 2014-12-05  3:54 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417643369-20603-1-git-send-email-davidu@nvidia.com>

> On 03/12/14 23:49, David Ung wrote:
> > Add high resolution modes to vesa_modes struct.
> >
> > Signed-off-by: David Ung <davidu@nvidia.com>
> > ---
> >  drivers/video/fbdev/core/modedb.c | 27 +++++++++++++++++++++++++++
> >  include/linux/fb.h                |  2 +-
> >  2 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/core/modedb.c
> > b/drivers/video/fbdev/core/modedb.c
> > index 388f797..0b57c1df 100644
> > --- a/drivers/video/fbdev/core/modedb.c
> > +++ b/drivers/video/fbdev/core/modedb.c
> > @@ -468,6 +468,33 @@ const struct fb_videomode vesa_modes[] = {
> >  	/* 33 1920x1440-75 VESA */
> >  	{ NULL, 75, 1920, 1440, 3367, 352, 144, 56, 1, 224, 3,
> >  	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 34 1920x1200-60 RB VESA */
> > +	{ NULL, 60, 1920, 1200, 6493, 80, 48, 26, 3, 32, 6,
> > +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 35 1920x1200-60 VESA */
> > +	{ NULL, 60, 1920, 1200, 5174, 336, 136, 36, 3, 200, 6,
> > +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 36 1920x1200-75 VESA */
> > +	{ NULL, 75, 1920, 1200, 4077, 344, 136, 46, 3, 208, 6,
> > +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 37 1920x1200-85 VESA */
> > +	{ NULL, 85, 1920, 1200, 3555, 352, 144, 53, 3, 208, 6,
> > +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 38 2560x1600-60 RB VESA */
> > +	{ NULL, 60, 2560, 1600, 3724, 80, 48, 37, 3, 32, 6,
> > +	  FB_SYNC_HOR_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 39 2560x1600-60 VESA */
> > +	{ NULL, 60, 2560, 1600, 2869, 472, 192, 49, 3, 280, 6,
> > +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 40 2560x1600-75 VESA */
> > +	{ NULL, 75, 2560, 1600, 2256, 488, 208, 63, 3, 280, 6,
> > +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 41 2560x1600-85 VESA */
> > +	{ NULL, 85, 2560, 1600, 1979, 488, 208, 73, 3, 280, 6,
> > +	  FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > +	/* 42 2560x1600-120 RB VESA */
> > +	{ NULL, 120, 2560, 1600, 1809, 80, 48, 85, 3, 32, 6,
> > +	  FB_SYNC_HOR_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> 
> Where did you take these timings? Are the modes in vesa_modes[] in some
> defined order, or just in the order they have been added?

From the DMT doc.  In the doc, the VESA modes are ordered by the DMT ids per page.
Each DMT mode is a valid vesa_mode.   vesa_modes are just a collection of modes
people had added over time.  There are quite a few modes that's missing from 
vesa_modes list.   Ideally the number of vesa_modes should match the number of
DMT ids.  If we wish to change the ordering, then some of the other drivers which
hardcodes the an index into vesa_modes will all need to be fixed.

David

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply

* RE: [PATCH 2/3] video: fbdev: Check Standard Timing against DMT
From: David Ung @ 2014-12-05  4:08 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417643369-20603-2-git-send-email-davidu@nvidia.com>

> > diff --git a/drivers/video/fbdev/core/modedb.c
> > b/drivers/video/fbdev/core/modedb.c
> > index 0b57c1df..858a97e 100644
> > --- a/drivers/video/fbdev/core/modedb.c
> > +++ b/drivers/video/fbdev/core/modedb.c
> > @@ -497,6 +497,90 @@ const struct fb_videomode vesa_modes[] = {
> >  	  FB_SYNC_HOR_HIGH_ACT, FB_VMODE_NONINTERLACED,
> FB_MODE_IS_VESA },
> > };  EXPORT_SYMBOL(vesa_modes);
> > +
> > +const struct dmt_videomode dmt_modes[DMT_SIZE] = {
> > +	{ 0x01, 0x0000, 0x000000, &vesa_modes[0] },
> > +	{ 0x02, 0x3119, 0x000000, &vesa_modes[1] },
> > +	{ 0x03, 0x0000, 0x000000, &vesa_modes[2] },
> > +	{ 0x04, 0x3140, 0x000000, &vesa_modes[3] },
> > +	{ 0x05, 0x314c, 0x000000, &vesa_modes[4] },
> > +	{ 0x06, 0x314f, 0x000000, &vesa_modes[5] },
> > +	{ 0x07, 0x3159, 0x000000, &vesa_modes[6] },
> > +	{ 0x08, 0x0000, 0x000000, &vesa_modes[7] },
> > +	{ 0x09, 0x4540, 0x000000, &vesa_modes[8] },
> > +	{ 0x0a, 0x454c, 0x000000, &vesa_modes[9] },
> > +	{ 0x0b, 0x454f, 0x000000, &vesa_modes[10] },
> > +	{ 0x0c, 0x4559, 0x000000, &vesa_modes[11] },
> > +	{ 0x0d, 0x0000, 0x000000, 0 },
> > +	{ 0x0e, 0x0000, 0x000000, 0 },
> 
> You've filled only some of the modes in this table. What's the logic which
> modes are left out?
> 

For DMT id 0xd, it has no STD 2byte id, no 3byte CVT code and no mode timings
currently defined in vesa_modes struct.
There is 80 DMT ids, but only 43 vesa_modes defined in fbdev.  So I've left those
entries empty.  If we eventually have all the VESA timings, the last column could
be eliminated.

David 

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply

* [PATCH] video: ocfb: Fix data type warning
From: Qiang Chen @ 2014-12-05  8:18 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
	Daniel Vetter, Laurent Pinchart, Stefan Kristiansson
  Cc: linux-fbdev, linux-kernel, Qiang Chen

When allocate framebuffer memory using dma_alloc_coherent(),
we'd better use dma_addr_t instead of phys_addr_t. Because the
address we got in fact is DMA or bus address for the platform.

This patch also fixes below build warning:
drivers/video/fbdev/ocfb.c:335:2:
	warning: passing argument 3 of ‘dma_alloc_attrs’
	from incompatible pointer type [enabled by default]

Signed-off-by: Qiang Chen <qiang2.chen@sonymobile.com>
---
 drivers/video/fbdev/ocfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/ocfb.c b/drivers/video/fbdev/ocfb.c
index 7f9dc9b..de98196 100644
--- a/drivers/video/fbdev/ocfb.c
+++ b/drivers/video/fbdev/ocfb.c
@@ -61,7 +61,7 @@ struct ocfb_dev {
 	/* flag indicating whether the regs are little endian accessed */
 	int little_endian;
 	/* Physical and virtual addresses of framebuffer */
-	phys_addr_t fb_phys;
+	dma_addr_t fb_phys;
 	void __iomem *fb_virt;
 	u32 pseudo_palette[PALETTE_SIZE];
 };
-- 
1.8.2.2


^ permalink raw reply related

* Re: [PATCH 2/3] video: fbdev: Check Standard Timing against DMT
From: Tomi Valkeinen @ 2014-12-05 11:41 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417643369-20603-2-git-send-email-davidu@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

On 05/12/14 06:08, David Ung wrote:
>>> diff --git a/drivers/video/fbdev/core/modedb.c
>>> b/drivers/video/fbdev/core/modedb.c
>>> index 0b57c1df..858a97e 100644
>>> --- a/drivers/video/fbdev/core/modedb.c
>>> +++ b/drivers/video/fbdev/core/modedb.c
>>> @@ -497,6 +497,90 @@ const struct fb_videomode vesa_modes[] = {
>>>  	  FB_SYNC_HOR_HIGH_ACT, FB_VMODE_NONINTERLACED,
>> FB_MODE_IS_VESA },
>>> };  EXPORT_SYMBOL(vesa_modes);
>>> +
>>> +const struct dmt_videomode dmt_modes[DMT_SIZE] = {
>>> +	{ 0x01, 0x0000, 0x000000, &vesa_modes[0] },
>>> +	{ 0x02, 0x3119, 0x000000, &vesa_modes[1] },
>>> +	{ 0x03, 0x0000, 0x000000, &vesa_modes[2] },
>>> +	{ 0x04, 0x3140, 0x000000, &vesa_modes[3] },
>>> +	{ 0x05, 0x314c, 0x000000, &vesa_modes[4] },
>>> +	{ 0x06, 0x314f, 0x000000, &vesa_modes[5] },
>>> +	{ 0x07, 0x3159, 0x000000, &vesa_modes[6] },
>>> +	{ 0x08, 0x0000, 0x000000, &vesa_modes[7] },
>>> +	{ 0x09, 0x4540, 0x000000, &vesa_modes[8] },
>>> +	{ 0x0a, 0x454c, 0x000000, &vesa_modes[9] },
>>> +	{ 0x0b, 0x454f, 0x000000, &vesa_modes[10] },
>>> +	{ 0x0c, 0x4559, 0x000000, &vesa_modes[11] },
>>> +	{ 0x0d, 0x0000, 0x000000, 0 },
>>> +	{ 0x0e, 0x0000, 0x000000, 0 },
>>
>> You've filled only some of the modes in this table. What's the logic which
>> modes are left out?
>>
> 
> For DMT id 0xd, it has no STD 2byte id, no 3byte CVT code and no mode timings
> currently defined in vesa_modes struct.
> There is 80 DMT ids, but only 43 vesa_modes defined in fbdev.  So I've left those
> entries empty.  If we eventually have all the VESA timings, the last column could
> be eliminated.

Ok. So you added the modes to vesa_modes table that you were interested
in for your use case, and then filled the dmt_modes table with the modes
that were available in vesa_modes?

That's ok, I just want to understand the logic for which modes you added
and which you left out.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] video: fbdev: Check Standard Timing against DMT
From: Tomi Valkeinen @ 2014-12-05 12:02 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417643369-20603-2-git-send-email-davidu@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 2100 bytes --]

On 03/12/14 23:49, David Ung wrote:
> Add the VESA Display Monitor Timing (DMT) table.
> During parsing of Standard Timings, it compare the 2 byte STD code
> with DMT to see what the VESA mode should be.  If there is no entry
> in the vesa_modes table or no match found, it fallsback to the
> GTF timings.
> 
> Signed-off-by: David Ung <davidu@nvidia.com>
> ---
>  drivers/video/fbdev/core/fbmon.c  | 20 ++++++----
>  drivers/video/fbdev/core/modedb.c | 84 +++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h                | 10 +++++
>  3 files changed, 107 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
> index 5b0e313..aa1110a 100644
> --- a/drivers/video/fbdev/core/fbmon.c
> +++ b/drivers/video/fbdev/core/fbmon.c
> @@ -526,16 +526,22 @@ static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
>  	refresh = (block[1] & 0x3f) + 60;
>  
>  	DPRINTK("      %dx%d@%dHz\n", xres, yres, refresh);
> -	for (i = 0; i < VESA_MODEDB_SIZE; i++) {
> -		if (vesa_modes[i].xres == xres &&
> -		    vesa_modes[i].yres == yres &&
> -		    vesa_modes[i].refresh == refresh) {
> -			*mode = vesa_modes[i];
> +	for (i = 0; i < DMT_SIZE; i++) {
> +		u32 std_2byte_code = block[0] << 8 | block[1];
> +
> +		if (std_2byte_code == dmt_modes[i].std_2byte_code) {
> +			if (!dmt_modes[i].mode)
> +				break;
> +			*mode = *dmt_modes[i].mode;
>  			mode->flag |= FB_MODE_IS_STANDARD;
> -			return 1;
> +			DPRINTK("        DMT id=%d\n", dmt_modes[i].dmt_id);
> +			break;
>  		}
>  	}
> -	calc_mode_timings(xres, yres, refresh, mode);
> +
> +	if (i == DMT_SIZE || !dmt_modes[i].mode)
> +		calc_mode_timings(xres, yres, refresh, mode);
> +
>  	return 1;
>  }

I think this could be made a bit cleaner.

The xres/yres/refresh calculation in get_std_timing doesn't matter for
the DMT code above. So in get_std_timing() you could first do the search
for the DMT mode, and if found, return from the function. After that the
code would do the GTF calculation.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox