* Re: [PATCH] Added backlight driver for Acer Aspire 4736
From: Pradeep Subrahmanion @ 2012-03-18 5:24 UTC (permalink / raw)
To: joeyli
Cc: Matthew Garrett, rpurdie, FlorianSchandinat, akpm, linux-fbdev,
linux-kernel
In-Reply-To: <1331798740.10557.245.camel@linux-s257.site>
On Thu, 2012-03-15 at 16:05 +0800, joeyli wrote:
> Hi Pradeep,
>
> 於 三，2012-03-14 於 11:47 +0530，Pradeep Subrahmanion 提到：
> > On Wed, Mar 14, 2012 at 11:21 AM, joeyli <jlee@suse.com> wrote:
> > > 於 三，2012-03-14 於 08:13 +0530，Pradeep Subrahmanion 提到：
> > >> Hi Joey ,
> > >>
> > >> > Per my understood, EC firmware should change brightness but didn't do
> > >> > that, another
> > >> > way is touch i915 register in _BCM.
> > >>
> > >> how do we do this ? you mean change the _BCM implementation ?
> > >
> > > "BIOS guy" should do something like this:
> > >
> > > Method (AINT, 2, NotSerialized)
> > > {
> > > ...
> > > If (LEqual (Arg0, One))
> > > {
> > > Store (Divide (Multiply (Arg1, 0xFF), 0x64, ), BCLP)
> > > Or (BCLP, 0x80000000, BCLP) <== touch BCLP register
> > > Store (0x02, ASLC)
> > > }
> > >
> > > Method (_BCM, 1, NotSerialized)
> > > {
> > > If (LAnd (LGreaterEqual (Arg0, Zero), LLessEqual (Arg0, 0x64)))
> > > {
> > > AINT (One, Arg0) <== call AINT method
> > > Store (Arg0, BRTL)
> > > }
> > > }
> > >
> > >
> > > Just for reference, they should do that when EC didn't wire to
> > > backlight.
> > >
> > >> >
> > >> > Acer machine provide a broken _BCM implementation and they didn't test
> > >> > it.
> > >> >
> > >> > > > > By ' ACPI interface' , I mean 'acpi_video0' inside the
> > >> > > > > /sys/class/backlight. I havn't tried the /sys/class/backlight interface
> > >> > > > > directly . I will try that also.
> > >> > > >
> > >> > > > So writing values into /sys/class/backlight/acpi_video0/brightness does
> > >> > > > nothing?
> > >> > >
> > >> > >
> > >> > > No change in value when writing
> > >> > > to /sys/class/backlight/acpi_video0/brightness.
> > >> > >
> > >> > > Another thing is that when i did boot with acpi_backlight = 'acer_wmi' ,
> > >> > > in new kernel (3.3.0-rc7) , it shows following messages ,
> > >> > >
> > >> > > [ 8.350825] wmi: Mapper loaded
> > >> > > [ 10.363975] acer_wmi: Acer Laptop ACPI-WMI Extras
> > >> > > [ 10.396186] acer_wmi: Function bitmap for Communication Device: 0x91
> > >> > > [ 10.396385] acer_wmi: Brightness must be controlled by generic video
> > >> > > driver
> > >> > >
> > >> > > Also there was no interface inside /sys/class/backlight for acer_wmi.
> > >> > >
> > >> >
> > >> > Yes, acer_wmi support backlight control with AMW0 interface, your
> > >> > machine didn't have AMW0 interface.
> > >> >
> > >> > Normally, backlight should control by standard acpi interface.
> > >> >
> > >> > > I also tried writing directly to Embedded controller register .But no
> > >> > > change.
> > >> >
> > >> > The machine has broken _BCM method, because EC should do something after
> > >> > _BCM changed EC register.
> > >>
> > >> Thanks ,
> > >>
> > >> Pradeep Subrahmanion
> > >
> > > Why they didn't find _BCM not work?
> > >
> > > My guess is:
> > >
> > > Because the backlight control is through WDDM driver on Windows platform
> > > but not through standard ACPI method _BCM. They only test Windows
> > > platform, so, they didn't find _BCM broken.
> > >
> > > And, they also didn't really follow Microsoft WDDM spec:
> > >
> > > http://msdn.microsoft.com/en-us/windows/hardware/gg487382.aspx
> > >
> > > Per spec,
> > > ODM should keep _BCM works fine for any other OS didn't support WDDM
> > > driver, but they didn't.
> > >
> > > At last year, I told Acer PM one time for this issue, they said will
> > > check but finally didn't response me.
> > >
> >
> > >
> > > Thanks
> > > Joey Lee
> > >
> >
> > So touching the PCI LBB register is the only feasible solution now
> > (even though it may not be a clean method) ?
> >
> > Thanks,
> > Pradeep Subrahmanion
>
> That will be better leave LBB register only touched by i915 driver.
>
> If 'acpi_backlight=vendor' works to you, maybe we can add a quirk to
> video_detect.c.
> You can try the following patch.
Thanks . I tried your patch .acpi_backlight=vendor allows me to control
brightness with hot key.But there are problems with it like increasing
brightness after maximum level causes blank screen.So I am trying it
sort it out.
Thank you ,
Pradeep Subrahmanion
>
>
> Thanks a lot!
> Joey Lee
>
>
> >From 038bd3c4e53b7195f34e9d46c999b8dcb279da5e Mon Sep 17 00:00:00 2001
> From: "Lee, Chun-Yi" <jlee@suse.com>
> Date: Thu, 15 Mar 2012 16:03:45 +0800
> Subject: [PATCH] acer-wmi: Add quirk table for video backlight vendor mode
>
> Add quirk table for video backlight vendor mode
>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> ---
> drivers/acpi/video_detect.c | 27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index f3f0fe7..acb15d6 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -132,6 +132,32 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
> return AE_OK;
> }
>
> +static int video_set_backlight_vendor(const struct dmi_system_id *d)
> +{
> + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> + return 0;
> +}
> +
> +static const struct dmi_system_id video_vendor_dmi_table[] = {
> + {
> + .callback = video_set_backlight_vendor,
> + .ident = "Acer Aspire 4736",
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 4736"),
> + },
> + },
> + {
> + .callback = video_set_backlight_vendor,
> + .ident = "Acer TravelMate 4750",
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 4750"),
> + },
> + },
> + {}
> +};
> +
> /*
> * Returns the video capabilities of a specific ACPI graphics device
> *
> @@ -164,6 +190,7 @@ long acpi_video_get_capabilities(acpi_handle graphics_handle)
> * ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> *}
> */
> + dmi_check_system(video_vendor_dmi_table);
> } else {
> status = acpi_bus_get_device(graphics_handle, &tmp_dev);
> if (ACPI_FAILURE(status)) {
^ permalink raw reply
* Re: [PATCH] Added backlight driver for Acer Aspire 4736
From: Pradeep Subrahmanion @ 2012-03-18 5:22 UTC (permalink / raw)
To: joeyli
Cc: Matthew Garrett, rpurdie, FlorianSchandinat, akpm, linux-fbdev,
linux-kernel
In-Reply-To: <1331798740.10557.245.camel@linux-s257.site>
On Thu, 2012-03-15 at 16:05 +0800, joeyli wrote:
> Hi Pradeep,
>
> 於 三，2012-03-14 於 11:47 +0530，Pradeep Subrahmanion 提到：
> > On Wed, Mar 14, 2012 at 11:21 AM, joeyli <jlee@suse.com> wrote:
> > > 於 三，2012-03-14 於 08:13 +0530，Pradeep Subrahmanion 提到：
> > >> Hi Joey ,
> > >>
> > >> > Per my understood, EC firmware should change brightness but didn't do
> > >> > that, another
> > >> > way is touch i915 register in _BCM.
> > >>
> > >> how do we do this ? you mean change the _BCM implementation ?
> > >
> > > "BIOS guy" should do something like this:
> > >
> > > Method (AINT, 2, NotSerialized)
> > > {
> > > ...
> > > If (LEqual (Arg0, One))
> > > {
> > > Store (Divide (Multiply (Arg1, 0xFF), 0x64, ), BCLP)
> > > Or (BCLP, 0x80000000, BCLP) <== touch BCLP register
> > > Store (0x02, ASLC)
> > > }
> > >
> > > Method (_BCM, 1, NotSerialized)
> > > {
> > > If (LAnd (LGreaterEqual (Arg0, Zero), LLessEqual (Arg0, 0x64)))
> > > {
> > > AINT (One, Arg0) <== call AINT method
> > > Store (Arg0, BRTL)
> > > }
> > > }
> > >
> > >
> > > Just for reference, they should do that when EC didn't wire to
> > > backlight.
> > >
> > >> >
> > >> > Acer machine provide a broken _BCM implementation and they didn't test
> > >> > it.
> > >> >
> > >> > > > > By ' ACPI interface' , I mean 'acpi_video0' inside the
> > >> > > > > /sys/class/backlight. I havn't tried the /sys/class/backlight interface
> > >> > > > > directly . I will try that also.
> > >> > > >
> > >> > > > So writing values into /sys/class/backlight/acpi_video0/brightness does
> > >> > > > nothing?
> > >> > >
> > >> > >
> > >> > > No change in value when writing
> > >> > > to /sys/class/backlight/acpi_video0/brightness.
> > >> > >
> > >> > > Another thing is that when i did boot with acpi_backlight = 'acer_wmi' ,
> > >> > > in new kernel (3.3.0-rc7) , it shows following messages ,
> > >> > >
> > >> > > [ 8.350825] wmi: Mapper loaded
> > >> > > [ 10.363975] acer_wmi: Acer Laptop ACPI-WMI Extras
> > >> > > [ 10.396186] acer_wmi: Function bitmap for Communication Device: 0x91
> > >> > > [ 10.396385] acer_wmi: Brightness must be controlled by generic video
> > >> > > driver
> > >> > >
> > >> > > Also there was no interface inside /sys/class/backlight for acer_wmi.
> > >> > >
> > >> >
> > >> > Yes, acer_wmi support backlight control with AMW0 interface, your
> > >> > machine didn't have AMW0 interface.
> > >> >
> > >> > Normally, backlight should control by standard acpi interface.
> > >> >
> > >> > > I also tried writing directly to Embedded controller register .But no
> > >> > > change.
> > >> >
> > >> > The machine has broken _BCM method, because EC should do something after
> > >> > _BCM changed EC register.
> > >>
> > >> Thanks ,
> > >>
> > >> Pradeep Subrahmanion
> > >
> > > Why they didn't find _BCM not work?
> > >
> > > My guess is:
> > >
> > > Because the backlight control is through WDDM driver on Windows platform
> > > but not through standard ACPI method _BCM. They only test Windows
> > > platform, so, they didn't find _BCM broken.
> > >
> > > And, they also didn't really follow Microsoft WDDM spec:
> > >
> > > http://msdn.microsoft.com/en-us/windows/hardware/gg487382.aspx
> > >
> > > Per spec,
> > > ODM should keep _BCM works fine for any other OS didn't support WDDM
> > > driver, but they didn't.
> > >
> > > At last year, I told Acer PM one time for this issue, they said will
> > > check but finally didn't response me.
> > >
> >
> > >
> > > Thanks
> > > Joey Lee
> > >
> >
> > So touching the PCI LBB register is the only feasible solution now
> > (even though it may not be a clean method) ?
> >
> > Thanks,
> > Pradeep Subrahmanion
>
> That will be better leave LBB register only touched by i915 driver.
>
> If 'acpi_backlight=vendor' works to you, maybe we can add a quirk to
> video_detect.c.
> You can try the following patch.
Thanks . I tried your patch .acpi_backlight=vendor allows me to control
brightness with hot key.But there are problems with it like increasing
brightness after maximum level causes blank screen.So I am trying it
sort it out.
Thank you ,
Pradeep Subrahmanion
>
>
> Thanks a lot!
> Joey Lee
>
>
> >From 038bd3c4e53b7195f34e9d46c999b8dcb279da5e Mon Sep 17 00:00:00 2001
> From: "Lee, Chun-Yi" <jlee@suse.com>
> Date: Thu, 15 Mar 2012 16:03:45 +0800
> Subject: [PATCH] acer-wmi: Add quirk table for video backlight vendor mode
>
> Add quirk table for video backlight vendor mode
>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> ---
> drivers/acpi/video_detect.c | 27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index f3f0fe7..acb15d6 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -132,6 +132,32 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
> return AE_OK;
> }
>
> +static int video_set_backlight_vendor(const struct dmi_system_id *d)
> +{
> + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> + return 0;
> +}
> +
> +static const struct dmi_system_id video_vendor_dmi_table[] = {
> + {
> + .callback = video_set_backlight_vendor,
> + .ident = "Acer Aspire 4736",
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 4736"),
> + },
> + },
> + {
> + .callback = video_set_backlight_vendor,
> + .ident = "Acer TravelMate 4750",
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Acer"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 4750"),
> + },
> + },
> + {}
> +};
> +
> /*
> * Returns the video capabilities of a specific ACPI graphics device
> *
> @@ -164,6 +190,7 @@ long acpi_video_get_capabilities(acpi_handle graphics_handle)
> * ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> *}
> */
> + dmi_check_system(video_vendor_dmi_table);
> } else {
> status = acpi_bus_get_device(graphics_handle, &tmp_dev);
> if (ACPI_FAILURE(status)) {
^ permalink raw reply
* Re: Problem with framebuffer mmap on platforms with large addressing
From: Benjamin Herrenschmidt @ 2012-03-18 0:46 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-fbdev-devel, Florian Tobias Schandinat, Tony Breeds,
linuxppc-dev
In-Reply-To: <CALT56yO05UBY6=KqK94=_sC4hd3ranj5J5CXOAo5hSjVAdkgkA@mail.gmail.com>
On Sat, 2012-03-17 at 20:04 +0400, Dmitry Eremin-Solenikov wrote:
> Hello,
>
> I'm trying to make framebuffer to work on PPC460EX board (canyonlands).
>
> The peculiarity of this platform is the fact that it has
> sizeof(unsigned long) = 4,
> but physical address on it is 36 bits width. It is a common to various pieces
> of the code to expect that unsigned long variable is able to contain physical
> address. Most of those places are easy to fix.
Yes. In fact, Tony (CC) has some patches to fix a lot of the DRM
infrastructure (we have radeon KMS working on a similar platform).
> The problem I'm stuck with is a fb_mmap() code. To find a right memory to map
> it uses information from struct fb_fix_screeninfo provided by the driver.
> This structure uses two unsigned long fields to hold physical addresses
> (smem_start and mmio_start). It would be easy to change that structure
> to use phys_addr_t instead of unsigned long, but this structure is a part
> of userspace ABI. It is returned to userspace on FBIOGET_FSCREENINFO ioctl.
> And now I'm stuck with it.
It's an old problem, which I think we described a while back on the
list. Back then the conclusion was to make a new version with a proper
__u64, a new ioctl to access is, and a "compatible" ioctl that blanks
the address fields (or fails) if they contain a value >32-bit.
We just never got to actually implement it.
In fact, we could make the new structure such that it doesn't break
userspace compatibility with 64-bit architectures at all, ie, the "new"
and "compat" ioctl could remain entirely equivalent on 64-bit.
> In my driver code I have just overwritten the fb_mmap function with
> driver-private
> fb_mmap callback supporting 64-bit addressing, but this doesn't look like
> a generic and correct solution.
>
> What is the best way to fix this problem? Should we break ABI with the goal
> of correctness? Should we add new FBIOGET_FSCREENINFO2, which will
> return a correct structure with phys_addr_t (or simply u64) fields and make
> FBIOGET_FSCREENINFO a wrapper returning partially bogus structure
> (with smem_start and mmio_start fields being truncated to just unsigned long)?
> What would developers recommend?
Cheers,
Ben.
^ permalink raw reply
* Fwd: Problem with framebuffer mmap on platforms with large addressing
From: Dmitry Eremin-Solenikov @ 2012-03-17 17:03 UTC (permalink / raw)
To: linux-fbdev
Hello,
I'm trying to make framebuffer to work on PPC460EX board (canyonlands).
The peculiarity of this platform is the fact that it has
sizeof(unsigned long) = 4,
but physical address on it is 36 bits width. It is a common to various pieces
of the code to expect that unsigned long variable is able to contain physical
address. Most of those places are easy to fix.
The problem I'm stuck with is a fb_mmap() code. To find a right memory to map
it uses information from struct fb_fix_screeninfo provided by the driver.
This structure uses two unsigned long fields to hold physical addresses
(smem_start and mmio_start). It would be easy to change that structure
to use phys_addr_t instead of unsigned long, but this structure is a part
of userspace ABI. It is returned to userspace on FBIOGET_FSCREENINFO ioctl.
And now I'm stuck with it.
In my driver code I have just overwritten the fb_mmap function with
driver-private
fb_mmap callback supporting 64-bit addressing, but this doesn't look like
a generic and correct solution.
What is the best way to fix this problem? Should we break ABI with the goal
of correctness? Should we add new FBIOGET_FSCREENINFO2, which will
return a correct structure with phys_addr_t (or simply u64) fields and make
FBIOGET_FSCREENINFO a wrapper returning partially bogus structure
(with smem_start and mmio_start fields being truncated to just unsigned long)?
What would developers recommend?
Thank you.
--
With best wishes
Dmitry
--
With best wishes
Dmitry
^ permalink raw reply
* Problem with framebuffer mmap on platforms with large addressing
From: Dmitry Eremin-Solenikov @ 2012-03-17 16:04 UTC (permalink / raw)
To: linux-fbdev-devel, linuxppc-dev, Florian Tobias Schandinat,
Josh Boyer, Matt Porter
Hello,
I'm trying to make framebuffer to work on PPC460EX board (canyonlands).
The peculiarity of this platform is the fact that it has
sizeof(unsigned long) = 4,
but physical address on it is 36 bits width. It is a common to various pieces
of the code to expect that unsigned long variable is able to contain physical
address. Most of those places are easy to fix.
The problem I'm stuck with is a fb_mmap() code. To find a right memory to map
it uses information from struct fb_fix_screeninfo provided by the driver.
This structure uses two unsigned long fields to hold physical addresses
(smem_start and mmio_start). It would be easy to change that structure
to use phys_addr_t instead of unsigned long, but this structure is a part
of userspace ABI. It is returned to userspace on FBIOGET_FSCREENINFO ioctl.
And now I'm stuck with it.
In my driver code I have just overwritten the fb_mmap function with
driver-private
fb_mmap callback supporting 64-bit addressing, but this doesn't look like
a generic and correct solution.
What is the best way to fix this problem? Should we break ABI with the goal
of correctness? Should we add new FBIOGET_FSCREENINFO2, which will
return a correct structure with phys_addr_t (or simply u64) fields and make
FBIOGET_FSCREENINFO a wrapper returning partially bogus structure
(with smem_start and mmio_start fields being truncated to just unsigned long)?
What would developers recommend?
Thank you.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] OMAPDSS: DISPC: Handle synclost errors in OMAP3
From: Mahapatra, Chandrabhanu @ 2012-03-16 14:27 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap
In-Reply-To: <1331893411.1653.5.camel@lappy>
2012/3/16 Tomi Valkeinen <tomi.valkeinen@ti.com>:
> On Fri, 2012-03-16 at 01:04 +0200, Ville Syrjälä wrote:
>> On Thu, Mar 15, 2012 at 05:18:52PM +0530, Chandrabhanu Mahapatra wrote:
>> > In OMAP3 DISPC video overlays suffer from some undocumented horizontal position
>> > and timing related limitations leading to SYNCLOST errors. Whenever the image
>> > window is moved towards the right of the screen SYNCLOST errors become
>> > frequent. Checks have been implemented to see that DISPC driver rejects
>> > configuration exceeding above limitations.
>> >
>> > This code was successfully tested on OMAP3. This code is written based on code
>> > written by Ville Syrjälä <ville.syrjala@nokia.com> in Linux OMAP kernel. Ville
>> > Syrjälä <ville.syrjala@nokia.com> had added checks for video overlay horizontal
>> > timing and DISPC horizontal blanking length limitations.
>> >
>> > Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> > ---
>> > drivers/video/omap2/dss/dispc.c | 67 +++++++++++++++++++++++++++++++++++++--
>> > 1 files changed, 64 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> > index 5a1963e..ebfa613 100644
>> > --- a/drivers/video/omap2/dss/dispc.c
>> > +++ b/drivers/video/omap2/dss/dispc.c
>> > @@ -1622,6 +1622,58 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
>> > }
>> > }
>> >
>> > +static int check_horiz_timing(enum omap_channel channel, u16 pos_x,
>> > + u16 width, u16 height, u16 out_width, u16 out_height)
>> > +{
>> > + int DS = DIV_ROUND_UP(height, out_height);
>> > + struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
>> > + struct omap_video_timings t = dssdev->panel.timings;
>> > + int pcd = REG_GET(DISPC_DIVISORo(channel), 7, 0);
>>
>> pcd doesn't exist for TV out, which is the reason why you see
>> 'lclk/pclk' ratio used instead in the harmattan kernel.
>
> I'm surprised you still remember these things =). And even more
> surprised that you actually bothered to review it, thanks for that.
>
> Chandrabhanu, do you have an omap3 board with composite/svideo tv-out?
> It'd be good to test on that also, as, like Ville points out, the above
> code can't work for tv-out. Hmm, then again, it shouldn't work for HDMI
> on OMAP4 either. Did you test with HDMI?
>
> Or is the above code called for TV-manager at all? If not, perhaps it
> could be named *_lcd or something. Or perhaps have a BUG_ON(channel !> lcd1 or lcd2);
>
Yes, I do have a Beagle board and I have tested on HDMI which works
without synclost errors upto 640 * 480 resolution.
The above errata exists only upto OMAP4430 ES1 which is currently not
supported. Please refer to errata 2.14 Downscaling Limitations in
errata sheet.
This patch is only for Omap3.
> Tomi
>
>
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
^ permalink raw reply
* Re: [PATCH 2/2] OMAPDSS: DISPC: Handle synclost errors in OMAP3
From: Mahapatra, Chandrabhanu @ 2012-03-16 14:18 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: linux-fbdev, linux-omap
In-Reply-To: <20120315230446.GO17132@sci.fi>
On Fri, Mar 16, 2012 at 4:34 AM, Ville Syrjälä <syrjala@sci.fi> wrote:
> On Thu, Mar 15, 2012 at 05:18:52PM +0530, Chandrabhanu Mahapatra wrote:
>> In OMAP3 DISPC video overlays suffer from some undocumented horizontal position
>> and timing related limitations leading to SYNCLOST errors. Whenever the image
>> window is moved towards the right of the screen SYNCLOST errors become
>> frequent. Checks have been implemented to see that DISPC driver rejects
>> configuration exceeding above limitations.
>>
>> This code was successfully tested on OMAP3. This code is written based on code
>> written by Ville Syrjälä <ville.syrjala@nokia.com> in Linux OMAP kernel. Ville
>> Syrjälä <ville.syrjala@nokia.com> had added checks for video overlay horizontal
>> timing and DISPC horizontal blanking length limitations.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>> drivers/video/omap2/dss/dispc.c | 67 +++++++++++++++++++++++++++++++++++++--
>> 1 files changed, 64 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 5a1963e..ebfa613 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -1622,6 +1622,58 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
>> }
>> }
>>
>> +static int check_horiz_timing(enum omap_channel channel, u16 pos_x,
>> + u16 width, u16 height, u16 out_width, u16 out_height)
>> +{
>> + int DS = DIV_ROUND_UP(height, out_height);
>> + struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
>> + struct omap_video_timings t = dssdev->panel.timings;
>> + int pcd = REG_GET(DISPC_DIVISORo(channel), 7, 0);
>
> pcd doesn't exist for TV out, which is the reason why you see
> 'lclk/pclk' ratio used instead in the harmattan kernel.
>
Yes, you are right. It's a very important point which I had missed.
> Another thing which still seems to be wrong in the upstream code is the
> use of fclk in the scaling checks. It should be checking lclk instead.
Thanks for sighting this out. I will implement it in the future.
>> + unsigned long nonactive, val, blank;
>> + static const u8 limits[3] = { 8, 10, 20 };
>> + int i;
>> +
>> + nonactive = t.x_res + t.hfp + t.hsw + t.hbp - out_width;
>> +
>> + /*
>> + * Atleast DS-2 lines must have already been fetched
>> + * before the display active video period starts.
>> + */
>> + val = (nonactive - pos_x) * pcd;
>> + DSSDBG("(nonactive - pos_x) * pcd = %lu,"
>> + " max(0, DS - 2) * width = %d\n",
>> + val, max(0, DS - 2) * width);
>> + if (val < max(0, DS - 2) * width)
>> + return -EINVAL;
>> +
>> + /*
>> + * Only one line can be fetched during the overlay active
>> + * period, the rest have to be fetched during the inactive
>> + * period.
>> + */
>> + val = nonactive * pcd;
>> + DSSDBG("nonactive * pcd = %lu, max(0, DS - 1) * width = %d\n",
>> + val, max(0, DS - 1) * width);
>> + if (val < max(0, DS - 1) * width)
>> + return -EINVAL;
>> +
>> + /*
>> + * Atleast Ceil(DS) lines should have been loaded during
>> + * PPL (screen width) + blanking period.
>> + */
>> + i = 0;
>> + if (out_height < height)
>> + i++;
>> + if (out_width < width)
>> + i++;
>> + blank = (t.hbp + t.hsw + t.hfp) * pcd;
>> + DSSDBG("blanking period + ppl = %lu (limit = %u)\n", blank, limits[i]);
>> + if (blank <= limits[i])
>> + return -EINVAL;
>
> The comment and code here are totally out of sync. IIRC the hblank
> length check came directly from the TRM. I have no idea if it's correct
> for more recent OMAPs, or if it was just copy pasted from the
> old TRM when the TRM got adjusted for more recent chips. That is why I
> stuck it into a separate function in harmattan.
>
This patch is only for OMAP3 and this errata is present upto OMAP4 ES1
which we are currently not supporting.
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
^ permalink raw reply
* Re: [PATCH 1/2] OMAPDSS: DISPC: Enable predecimation
From: Mahapatra, Chandrabhanu @ 2012-03-16 12:11 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: linux-fbdev, linux-omap
In-Reply-To: <20120315222811.GN17132@sci.fi>
On Fri, Mar 16, 2012 at 3:58 AM, Ville Syrjälä <syrjala@sci.fi> wrote:
> On Thu, Mar 15, 2012 at 05:18:28PM +0530, Chandrabhanu Mahapatra wrote:
>> In OMAP3 and OMAP4, the DISPC Scaler can downscale an image up to 4 times, and
>> up to 2 times in OMAP2. However, with predecimation, the image can be reduced
>> to 16 times by fetching only the necessary pixels in memory. Then this
>> predecimated image can be downscaled further by the DISPC scaler.
>
> Now, where does that number 16 come from? IIRC the hardware can skip
> basically any number of pixels/rows. I certainly didn't add any such
> limit to the code in the harmattan kernel, and distinctly remember
> being able to downscale the N9/N950 UI even down to 1 pixel size :)
>
Yes, you are right. I have done vertical predecimation to a size of 2
lines beyond which it failed.
This limitation of 16 is actually on horizontal predecimation as was
mentioned by the hardware guys. Let me explain what we once discussed
with Archit.
The pipeline is configured to use a burst of size 8 * 128 bits and DSS
uses 8 L3 clock cycles to fetch it. So, burst consists of 8 mini
bursts of 16 bytes each.
Now, there are two possible situations:
1) Each mini burst has some valid pixels, so all 16 bytes are fetched
by DISPC DMA and the uneeded pixels are discarded. This case can be
shown as
pixelinc -1 + bpp =< 16
2) Some mini bursts don't have any valid pixels, so these will not be
fecthed by the DISPC DMA. So, L3 interconnect may handover the bus
over so some other initiator and inturn delay the fecthing of pixels
leading to UNDERFLOWS. This case can be shown as
pixellinc -1 + bpp > 16.
To avoid these UNDERFLOWS we better have a limitation of 16 on
horizontal predecimation. Anyways, 16 times predecimation followed by
2/4 times downscaling (may it be horizontal or vertical) by DISPC
SCALER is more than enough for any usecase required.
>> Based on the downscaling required, a prior calculation of predecimation values
>> for width and height of an image is done. Since, Predecimation reduces quality
>> of an image higher priorty is given to DISPC Scaler for downscaling.
>>
>> This code was successfully tested on OMAP2, OMAP3 and OMAP4. Horizontal and
>> vertical predecimation worked fine except for some synclost errors due to
>> undocumented errata in OMAP3 which are fixed later and skewed images were seen
>> on OMAP2 and OMAP3 during horizontal predecimation which will be addressed in
>> the future patches.
>
> All the rotation offset calculations still look suspiciously different
> to what is in the harmattan kernel. I remember that the original code
> was quite broken, and I fixed a lot of things when I was implementing
> pre-decimation and some rotation stuff for the N9/N950. Too bad I never
> managed to push that stuff upstream...
>
A number of variables are different in mainline kernel from that used
in your code. I did fix some of the rotation code to get the best
results. I will consider your suggestions for further improvement.
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
^ permalink raw reply
* Re: [PATCH 2/2] OMAPDSS: DISPC: Handle synclost errors in OMAP3
From: Tomi Valkeinen @ 2012-03-16 10:23 UTC (permalink / raw)
To: Ville Syrjälä, Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <20120315230446.GO17132@sci.fi>
On Fri, 2012-03-16 at 01:04 +0200, Ville Syrjälä wrote:
> On Thu, Mar 15, 2012 at 05:18:52PM +0530, Chandrabhanu Mahapatra wrote:
> > In OMAP3 DISPC video overlays suffer from some undocumented horizontal position
> > and timing related limitations leading to SYNCLOST errors. Whenever the image
> > window is moved towards the right of the screen SYNCLOST errors become
> > frequent. Checks have been implemented to see that DISPC driver rejects
> > configuration exceeding above limitations.
> >
> > This code was successfully tested on OMAP3. This code is written based on code
> > written by Ville Syrjälä <ville.syrjala@nokia.com> in Linux OMAP kernel. Ville
> > Syrjälä <ville.syrjala@nokia.com> had added checks for video overlay horizontal
> > timing and DISPC horizontal blanking length limitations.
> >
> > Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> > ---
> > drivers/video/omap2/dss/dispc.c | 67 +++++++++++++++++++++++++++++++++++++--
> > 1 files changed, 64 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> > index 5a1963e..ebfa613 100644
> > --- a/drivers/video/omap2/dss/dispc.c
> > +++ b/drivers/video/omap2/dss/dispc.c
> > @@ -1622,6 +1622,58 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
> > }
> > }
> >
> > +static int check_horiz_timing(enum omap_channel channel, u16 pos_x,
> > + u16 width, u16 height, u16 out_width, u16 out_height)
> > +{
> > + int DS = DIV_ROUND_UP(height, out_height);
> > + struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
> > + struct omap_video_timings t = dssdev->panel.timings;
> > + int pcd = REG_GET(DISPC_DIVISORo(channel), 7, 0);
>
> pcd doesn't exist for TV out, which is the reason why you see
> 'lclk/pclk' ratio used instead in the harmattan kernel.
I'm surprised you still remember these things =). And even more
surprised that you actually bothered to review it, thanks for that.
Chandrabhanu, do you have an omap3 board with composite/svideo tv-out?
It'd be good to test on that also, as, like Ville points out, the above
code can't work for tv-out. Hmm, then again, it shouldn't work for HDMI
on OMAP4 either. Did you test with HDMI?
Or is the above code called for TV-manager at all? If not, perhaps it
could be named *_lcd or something. Or perhaps have a BUG_ON(channel !lcd1 or lcd2);
Tomi
^ permalink raw reply
* Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
From: Darius Augulis @ 2012-03-16 10:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <002f01cd0359$eb718330$c2548990$%han@samsung.com>
On Fri, Mar 16, 2012 at 11:48 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>> > It's just bug.
>> >
>> > struct s3c_fb_pd_win should be used for windows of FIMD IP.
>> > It should not be used for selecting multi-LCDs.
>>
>> why not? s3c_fb_pd_win contains timing values which depend directly on
>> LCD size and resolution.
>> Please look into the code again - mini6410 doesn't use platform data
>> so select different LCD sizes.
>> It has only single window in platform data - exactly what you want.
>> Now tell my, why you are arguing, that this platform data cannot be
>> rewritten dynamically by board setup routines at kernel startup time?
>> Many ARM architectures are doing the same - takes kernel command line
>> argument to select LCD and passes necessary data for its fb drivers
>> via platform data structures. It not a bug, it's correct approach to
>> support different sizes of LCD display for the same board.
>
>
> Hey, it is not my point.
> To take kernel command line is not problem to support multiple LCDs.
>
> My point is that s3c_fb_pd_win should not include variable LCD option.
> So, if you want to support multiple LCDs, you should use another variable,
> instead of using s3c_fb_pd_win[0] and s3c_fb_pd_win[1] as 4.3" LCD, 7.0" LCD.
But I use s3c_fb_pd_win only as container to hold timing values in
board setup code.
s3c-fb platform data has only single window and does not perform any
LCD selection.
What is wrong here? You suggest to declare some custom structure to
hold these timing values? What's the point?
>
> You use s3c_fb_pd_win[0], s3c_fb_pd_win[1] as 4.3" LCD, 7.0" LCD.
> However, s3c_fb_pd_win[0], s3c_fb_pd_win[1] means the windows of FIMD IP.
There is no multiple windows in platform data! driver sees only single
window. And this wont change in any way if you drop LCD selection from
board setup code.
Driver will see the same single window. So - your are arguing, that
board setup code sets platform data in bad way. I don't accept your
opinion, because tens of boards are doing the same and nobody call it
bug.
>
^ permalink raw reply
* RE: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
From: Jingoo Han @ 2012-03-16 9:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAEYLNCpfQ0cYb4efw6PZNnKjQ2rF25rj5Ed=i884LJXHAwigBw@mail.gmail.com>
> -----Original Message-----
> From: Darius Augulis [mailto:augulis.darius@gmail.com]
> Sent: Friday, March 16, 2012 6:16 PM
> To: Jingoo Han
> Cc: Thomas Abraham; linux-fbdev@vger.kernel.org; FlorianSchandinat@gmx.de; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; ben-linux@fluff.org; patches@linaro.org;
> kgene.kim@samsung.com; Kyungmin Park; JeongHyeon Kim; Heiko Stuebner; Kwangwoo Lee; Mark Brown; Peter
> Korsgaard; Maurus Cuelenaere; Tushar Behera
> Subject: Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
>
> On Fri, Mar 16, 2012 at 10:07 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Darius Augulis [mailto:augulis.darius@gmail.com]
> >> Sent: Friday, March 16, 2012 4:47 PM
> >> To: Jingoo Han
> >> Cc: Thomas Abraham; linux-fbdev@vger.kernel.org; FlorianSchandinat@gmx.de; linux-arm-
> >> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; ben-linux@fluff.org;
> patches@linaro.org;
> >> kgene.kim@samsung.com; Kyungmin Park; JeongHyeon Kim; Heiko Stuebner; Kwangwoo Lee; Mark Brown; Peter
> >> Korsgaard; Maurus Cuelenaere; Tushar Behera
> >> Subject: Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
> >>
> >> On Fri, Mar 16, 2012 at 3:39 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> >>
> >> it's not a bug, because it's working like it was intended to. it was
> >> reviewed and accepted by maintainers before merging to main line.
> >> you do not have rights to drop it because you don't like it.
> >> You are doing changes - please do it correctly and do not break
> >> existing functionality.
> >> btw, you still not answered my question: why these two s3c_fb_pd_win
> >> structures in mach-mini6410.c is a problem? They are declared on the
> >> stack, but platform data uses only single one. mini6410 rewrites its
> >> s3c-fb platform data with data from one of these two structures
> >> dynamically. Why it's a bug? You want to remove this dynamic selection
> >> which could be leaved there with reworked framework too.
> >
> >
> > It's just bug.
> >
> > struct s3c_fb_pd_win should be used for windows of FIMD IP.
> > It should not be used for selecting multi-LCDs.
>
> why not? s3c_fb_pd_win contains timing values which depend directly on
> LCD size and resolution.
> Please look into the code again - mini6410 doesn't use platform data
> so select different LCD sizes.
> It has only single window in platform data - exactly what you want.
> Now tell my, why you are arguing, that this platform data cannot be
> rewritten dynamically by board setup routines at kernel startup time?
> Many ARM architectures are doing the same - takes kernel command line
> argument to select LCD and passes necessary data for its fb drivers
> via platform data structures. It not a bug, it's correct approach to
> support different sizes of LCD display for the same board.
Hey, it is not my point.
To take kernel command line is not problem to support multiple LCDs.
My point is that s3c_fb_pd_win should not include variable LCD option.
So, if you want to support multiple LCDs, you should use another variable,
instead of using s3c_fb_pd_win[0] and s3c_fb_pd_win[1] as 4.3" LCD, 7.0" LCD.
You use s3c_fb_pd_win[0], s3c_fb_pd_win[1] as 4.3" LCD, 7.0" LCD.
However, s3c_fb_pd_win[0], s3c_fb_pd_win[1] means the windows of FIMD IP.
> You are simply lazy to deal with little bit different code of mini6410
> and real6410 because they have support for multiple LCDs and other
> s3c-fb boards doesn't.
> This is a reason why you want to drop it and not because it's a bug.
> Hope other maintainers and code reviewers understand this situation correctly.
>
> >
> > struct s3c_fb_pd_win should include information of windows of FIMD IP, s3c_fb_pd_win[0],
> s3c_fb_pd_win[1] represents window 0,
> > window 1 of FIMD IP. However, your code includes information of LCD, it means that struct
> s3c_fb_pd_win represents 4.3" LCD, 7.0"
> > LCD. It is wrong usage.
> >
> >
> >>
> >> > 'struct s3c_fb_pd_win' is used not for LCD, but for windows of FIMD IP. So, 'struct s3c_fb_pd_win'
> >> should not be used to select LCD.
> >> > The mach-aquila.c uses two 'struct s3c_fb_pd_win' like you, however 'struct s3c_fb_pd_win' don't
> make
> >> problem with Thomas's
> >> > patchset. It is because mach-aquila.c use 'struct s3c_fb_pd_win' for only windows of FIMD IP.
> >> > We don't have to responsibility to keep wrong code. If you have kept the rule, Thomas's patch would
> >> not make the compatibility
> >> > problem with your code. Why we should keep aberration such as your code? If you want to select two
> >> LCDs, please find other way.
> >> >
> >> >
> >> >>
> >> >> Darius A.
> >> >
> >
^ permalink raw reply
* Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
From: Darius Augulis @ 2012-03-16 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <002d01cd034b$c56f2b00$504d8100$%han@samsung.com>
On Fri, Mar 16, 2012 at 10:07 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Darius Augulis [mailto:augulis.darius@gmail.com]
>> Sent: Friday, March 16, 2012 4:47 PM
>> To: Jingoo Han
>> Cc: Thomas Abraham; linux-fbdev@vger.kernel.org; FlorianSchandinat@gmx.de; linux-arm-
>> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; ben-linux@fluff.org; patches@linaro.org;
>> kgene.kim@samsung.com; Kyungmin Park; JeongHyeon Kim; Heiko Stuebner; Kwangwoo Lee; Mark Brown; Peter
>> Korsgaard; Maurus Cuelenaere; Tushar Behera
>> Subject: Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
>>
>> On Fri, Mar 16, 2012 at 3:39 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>>
>> it's not a bug, because it's working like it was intended to. it was
>> reviewed and accepted by maintainers before merging to main line.
>> you do not have rights to drop it because you don't like it.
>> You are doing changes - please do it correctly and do not break
>> existing functionality.
>> btw, you still not answered my question: why these two s3c_fb_pd_win
>> structures in mach-mini6410.c is a problem? They are declared on the
>> stack, but platform data uses only single one. mini6410 rewrites its
>> s3c-fb platform data with data from one of these two structures
>> dynamically. Why it's a bug? You want to remove this dynamic selection
>> which could be leaved there with reworked framework too.
>
>
> It's just bug.
>
> struct s3c_fb_pd_win should be used for windows of FIMD IP.
> It should not be used for selecting multi-LCDs.
why not? s3c_fb_pd_win contains timing values which depend directly on
LCD size and resolution.
Please look into the code again - mini6410 doesn't use platform data
so select different LCD sizes.
It has only single window in platform data - exactly what you want.
Now tell my, why you are arguing, that this platform data cannot be
rewritten dynamically by board setup routines at kernel startup time?
Many ARM architectures are doing the same - takes kernel command line
argument to select LCD and passes necessary data for its fb drivers
via platform data structures. It not a bug, it's correct approach to
support different sizes of LCD display for the same board.
You are simply lazy to deal with little bit different code of mini6410
and real6410 because they have support for multiple LCDs and other
s3c-fb boards doesn't.
This is a reason why you want to drop it and not because it's a bug.
Hope other maintainers and code reviewers understand this situation correctly.
>
> struct s3c_fb_pd_win should include information of windows of FIMD IP, s3c_fb_pd_win[0], s3c_fb_pd_win[1] represents window 0,
> window 1 of FIMD IP. However, your code includes information of LCD, it means that struct s3c_fb_pd_win represents 4.3" LCD, 7.0"
> LCD. It is wrong usage.
>
>
>>
>> > 'struct s3c_fb_pd_win' is used not for LCD, but for windows of FIMD IP. So, 'struct s3c_fb_pd_win'
>> should not be used to select LCD.
>> > The mach-aquila.c uses two 'struct s3c_fb_pd_win' like you, however 'struct s3c_fb_pd_win' don't make
>> problem with Thomas's
>> > patchset. It is because mach-aquila.c use 'struct s3c_fb_pd_win' for only windows of FIMD IP.
>> > We don't have to responsibility to keep wrong code. If you have kept the rule, Thomas's patch would
>> not make the compatibility
>> > problem with your code. Why we should keep aberration such as your code? If you want to select two
>> LCDs, please find other way.
>> >
>> >
>> >>
>> >> Darius A.
>> >
>
^ permalink raw reply
* RE: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
From: Jingoo Han @ 2012-03-16 8:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAEYLNCrGCmb5EJjURLY=EGfM=4N54MirNkCCyQRxqKEN3Eka8Q@mail.gmail.com>
> -----Original Message-----
> From: Darius Augulis [mailto:augulis.darius@gmail.com]
> Sent: Friday, March 16, 2012 4:47 PM
> To: Jingoo Han
> Cc: Thomas Abraham; linux-fbdev@vger.kernel.org; FlorianSchandinat@gmx.de; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; ben-linux@fluff.org; patches@linaro.org;
> kgene.kim@samsung.com; Kyungmin Park; JeongHyeon Kim; Heiko Stuebner; Kwangwoo Lee; Mark Brown; Peter
> Korsgaard; Maurus Cuelenaere; Tushar Behera
> Subject: Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
>
> On Fri, Mar 16, 2012 at 3:39 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>
> >> > So, your code can be removed.
> >>
> >> I don't think so. It does not break anything yet. One who make
> >> changes, should ensure backwards compatibility, at least talking about
> >> functionality and hardware (LCD) support, which was added few months
> >> ago. Remember, we talk about kernel parameters line - now it lets
> >> bootloader to select correct LCD size. After your changes, board with
> >> 7" LCD wont show anything.
> >
> > [CC'ed Tushar Behera]
> >
> > Yes, your code is working. However, your code is bug.
>
> it's not a bug, because it's working like it was intended to. it was
> reviewed and accepted by maintainers before merging to main line.
> you do not have rights to drop it because you don't like it.
> You are doing changes - please do it correctly and do not break
> existing functionality.
> btw, you still not answered my question: why these two s3c_fb_pd_win
> structures in mach-mini6410.c is a problem? They are declared on the
> stack, but platform data uses only single one. mini6410 rewrites its
> s3c-fb platform data with data from one of these two structures
> dynamically. Why it's a bug? You want to remove this dynamic selection
> which could be leaved there with reworked framework too.
It's just bug.
struct s3c_fb_pd_win should be used for windows of FIMD IP.
It should not be used for selecting multi-LCDs.
struct s3c_fb_pd_win should include information of windows of FIMD IP, s3c_fb_pd_win[0], s3c_fb_pd_win[1] represents window 0,
window 1 of FIMD IP. However, your code includes information of LCD, it means that struct s3c_fb_pd_win represents 4.3" LCD, 7.0"
LCD. It is wrong usage.
>
> > 'struct s3c_fb_pd_win' is used not for LCD, but for windows of FIMD IP. So, 'struct s3c_fb_pd_win'
> should not be used to select LCD.
> > The mach-aquila.c uses two 'struct s3c_fb_pd_win' like you, however 'struct s3c_fb_pd_win' don't make
> problem with Thomas's
> > patchset. It is because mach-aquila.c use 'struct s3c_fb_pd_win' for only windows of FIMD IP.
> > We don't have to responsibility to keep wrong code. If you have kept the rule, Thomas's patch would
> not make the compatibility
> > problem with your code. Why we should keep aberration such as your code? If you want to select two
> LCDs, please find other way.
> >
> >
> >>
> >> Darius A.
> >
^ permalink raw reply
* Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
From: Darius Augulis @ 2012-03-16 7:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <001d01cd0315$935cc720$ba165560$%han@samsung.com>
On Fri, Mar 16, 2012 at 3:39 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>> > So, your code can be removed.
>>
>> I don't think so. It does not break anything yet. One who make
>> changes, should ensure backwards compatibility, at least talking about
>> functionality and hardware (LCD) support, which was added few months
>> ago. Remember, we talk about kernel parameters line - now it lets
>> bootloader to select correct LCD size. After your changes, board with
>> 7" LCD wont show anything.
>
> [CC'ed Tushar Behera]
>
> Yes, your code is working. However, your code is bug.
it's not a bug, because it's working like it was intended to. it was
reviewed and accepted by maintainers before merging to main line.
you do not have rights to drop it because you don't like it.
You are doing changes - please do it correctly and do not break
existing functionality.
btw, you still not answered my question: why these two s3c_fb_pd_win
structures in mach-mini6410.c is a problem? They are declared on the
stack, but platform data uses only single one. mini6410 rewrites its
s3c-fb platform data with data from one of these two structures
dynamically. Why it's a bug? You want to remove this dynamic selection
which could be leaved there with reworked framework too.
> 'struct s3c_fb_pd_win' is used not for LCD, but for windows of FIMD IP. So, 'struct s3c_fb_pd_win' should not be used to select LCD.
> The mach-aquila.c uses two 'struct s3c_fb_pd_win' like you, however 'struct s3c_fb_pd_win' don't make problem with Thomas's
> patchset. It is because mach-aquila.c use 'struct s3c_fb_pd_win' for only windows of FIMD IP.
> We don't have to responsibility to keep wrong code. If you have kept the rule, Thomas's patch would not make the compatibility
> problem with your code. Why we should keep aberration such as your code? If you want to select two LCDs, please find other way.
>
>
>>
>> Darius A.
>
^ permalink raw reply
* Re: [PATCH] [#7001] framebuffer: old image of last app would retain for a while after starting new app
From: Mike Frysinger @ 2012-03-16 5:21 UTC (permalink / raw)
To: Zhang, Sonic; +Cc: uclinux-dist-devel, Wu, Aaron, linux-fbdev-devel
In-Reply-To: <DB904C5425BA6F4E8424B3B51A1414D16F24304D64@NWD2CMBX1.ad.analog.com>
[-- Attachment #1.1: Type: Text/Plain, Size: 2145 bytes --]
On Friday 16 March 2012 01:05:10 Zhang, Sonic wrote:
> From: Mike Frysinger [mailto:vapier@gentoo.org]
>> On Thursday 15 March 2012 23:34:08 Zhang, Sonic wrote:
>>> From: Mike Frysinger [mailto:vapier@gentoo.org]
>>>> On Thursday 15 March 2012 05:23:50 Wu, Aaron wrote:
>>>>> Old image of last application would retain for a while when
>>>>> starting a new application, this patch clear the frambuffer before
>>>>> displaying every time the fb is opened.
>>>>
>>>> i'm not sure the behavior you describe is wrong. in fact, i'm pretty
>>>> sure it sounds correct. if an app writes an image to the framebuffer
>>>> and then quits, that image should stay there indefinitely until
>>>> something else opens the framebuffer and draws their own image.
>>>
>>> Before the second application draws its own image, the image of last
>>> application has already been displayed on the LCD when the second
>>> application opens the FB device(PPI is enabled when it is opened).
>>> This is not a correct behavior.
>>
>> sure it is. if the app wants to clear the screen, it can do so. generally
>> it's going to anyways by drawing an entire frame. having the frame buffer
>> driver always clear the screen introduces wasted memory overhead as it
>> does the memset(), and user-visible jank as the device transitions from an
>> initial splash screen to the main userspace app.
>>
>> this is a policy decision that doesn't really belong in kernel space. and
>> if it did, it should be agreed upon by all frame buffer users and not just
>> changing a few drivers.
>
> How can the application clean the screen before it opens the FB device?
why does it need to be before open ? if the kernel does the memset or
userspace does the memset, the frame still gets cleared.
in looking at the blackfin framebuffer drivers, i'm not sure they're correct. i
don't think the PPI/DMA should be shutdown when the last user space client
closes it. fb_release is for releasing all resources when tearing down the
driver, and fb_blank is runtime management (turning off vsync/hsync and
powering down the screen).
-mike
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 191 bytes --]
------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
[-- Attachment #3: Type: text/plain, Size: 182 bytes --]
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
^ permalink raw reply
* Re: [PATCH] [#7001] framebuffer: old image of last app would retain for a while after starting new app
From: Zhang, Sonic @ 2012-03-16 5:05 UTC (permalink / raw)
To: Mike Frysinger
Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org,
Wu, Aaron,
linux-fbdev-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <201203152354.33365.vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
-----Original Message-----
From: Mike Frysinger [mailto:vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org]
Sent: Friday, March 16, 2012 11:55 AM
To: Zhang, Sonic
Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org; Wu, Aaron; linux-fbdev-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Subject: Re: [PATCH] [#7001] framebuffer: old image of last app would retain for a while after starting new app
On Thursday 15 March 2012 23:34:08 Zhang, Sonic wrote:
> From: Mike Frysinger [mailto:vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org]
>> On Thursday 15 March 2012 05:23:50 Wu, Aaron wrote:
>> > Old image of last application would retain for a while when
>> > starting a new application, this patch clear the frambuffer before
>> > displaying every time the fb is opened.
>>
>> i'm not sure the behavior you describe is wrong. in fact, i'm pretty
>> sure it sounds correct. if an app writes an image to the framebuffer
>> and then quits, that image should stay there indefinitely until
>> something else opens the framebuffer and draws their own image.
>
> Before the second application draws its own image, the image of last
> application has already been displayed on the LCD when the second
> application opens the FB device(PPI is enabled when it is opened).
> This is not a correct behavior.
sure it is. if the app wants to clear the screen, it can do so. generally it's going to anyways by drawing an entire frame. having the frame buffer driver always clear the screen introduces wasted memory overhead as it does the memset(), and user-visible jank as the device transitions from an initial splash screen to the main userspace app.
this is a policy decision that doesn't really belong in kernel space. and if it did, it should be agreed upon by all frame buffer users and not just changing a few drivers.
-mike
How can the application clean the screen before it opens the FB device?
Sonic
^ permalink raw reply
* Re: [PATCH] [#7001] framebuffer: old image of last app would retain for a while after starting new app
From: Mike Frysinger @ 2012-03-16 3:54 UTC (permalink / raw)
To: Zhang, Sonic; +Cc: uclinux-dist-devel, Wu, Aaron, linux-fbdev-devel
In-Reply-To: <DB904C5425BA6F4E8424B3B51A1414D16F24304D28@NWD2CMBX1.ad.analog.com>
[-- Attachment #1.1: Type: Text/Plain, Size: 1432 bytes --]
On Thursday 15 March 2012 23:34:08 Zhang, Sonic wrote:
> From: Mike Frysinger [mailto:vapier@gentoo.org]
>> On Thursday 15 March 2012 05:23:50 Wu, Aaron wrote:
>> > Old image of last application would retain for a while when starting a
>> > new application, this patch clear the frambuffer before displaying
>> > every time the fb is opened.
>>
>> i'm not sure the behavior you describe is wrong. in fact, i'm pretty sure
>> it sounds correct. if an app writes an image to the framebuffer and then
>> quits, that image should stay there indefinitely until something else
>> opens the framebuffer and draws their own image.
>
> Before the second application draws its own image, the image of last
> application has already been displayed on the LCD when the second
> application opens the FB device(PPI is enabled when it is opened). This is
> not a correct behavior.
sure it is. if the app wants to clear the screen, it can do so. generally
it's going to anyways by drawing an entire frame. having the frame buffer
driver always clear the screen introduces wasted memory overhead as it does
the memset(), and user-visible jank as the device transitions from an initial
splash screen to the main userspace app.
this is a policy decision that doesn't really belong in kernel space. and if
it did, it should be agreed upon by all frame buffer users and not just
changing a few drivers.
-mike
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 191 bytes --]
------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
[-- Attachment #3: Type: text/plain, Size: 182 bytes --]
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
^ permalink raw reply
* Re: [PATCH] [#7001] framebuffer: old image of last app would retain for a while after starting new app
From: Zhang, Sonic @ 2012-03-16 3:34 UTC (permalink / raw)
To: Mike Frysinger,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org
Cc: Wu, Aaron,
linux-fbdev-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <201203151508.04564.vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
-----Original Message-----
From: Mike Frysinger [mailto:vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org]
Sent: Friday, March 16, 2012 3:08 AM
To: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org
Cc: Wu, Aaron; linux-fbdev-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Zhang, Sonic
Subject: Re: [uclinux-dist-devel] [PATCH] [#7001] framebuffer: old image of last app would retain for a while after starting new app
On Thursday 15 March 2012 05:23:50 Wu, Aaron wrote:
> Old image of last application would retain for a while when starting a
> new application, this patch clear the frambuffer before displaying
> every time the fb is opened.
i'm not sure the behavior you describe is wrong. in fact, i'm pretty sure it sounds correct. if an app writes an image to the framebuffer and then quits, that image should stay there indefinitely until something else opens the framebuffer and draws their own image.
-mike
Before the second application draws its own image, the image of last application has already been displayed on the LCD when the second application opens the FB device(PPI is enabled when it is opened). This is not a correct behavior.
Sonic
^ permalink raw reply
* RE: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
From: Jingoo Han @ 2012-03-16 1:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAEYLNCrprPE60ZS=_OWuck0JCJMhCxzVErZwuwGpEDYkQq-hXg@mail.gmail.com>
> -----Original Message-----
> From: Darius Augulis [mailto:augulis.darius@gmail.com]
> Sent: Thursday, March 15, 2012 5:27 PM
> To: Jingoo Han
> Cc: Thomas Abraham; linux-fbdev@vger.kernel.org; FlorianSchandinat@gmx.de; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; kgene.kim@samsung.com; ben-
> linux@fluff.org; patches@linaro.org; Kyungmin Park; JeongHyeon Kim; Heiko Stuebner; Kwangwoo Lee; Mark
> Brown; Peter Korsgaard; Maurus Cuelenaere
> Subject: Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
>
> On Thu, Mar 15, 2012 at 10:10 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>
> >> There is single board - mini6410 (or real6410) and it's name doesn't
> >> depend on connected LCD size.
> >> We know, that this board is available with different sizes of LCD and
> >> currently we have in kernel support for both sizes.
> >> It might be so, that it's implemented not in perfect way, but it was
> >> accepted and at least it's working.
> >
> >
> > I don't think so.
> > You argues that the wrong code should not be removed because it was accepted and at least it's working.
> > It is just wrong usage, which can just work.
> > Moreover, your code will make the problem, when 2~5 windows of FIMD IP are used.
>
> Could please explain that? How does LCD feature selection for mini6410
> and real6410 break that?
> Timing for different sizes of LCD is located in two statically
> allocated structures which are used at board init time depending on
> kernel param line.
> Data from one of these structures is copied to s3c-fb platform data
> dynamically by board init code. Why it's a problem? What changes if
> platform data isn't rewritten dynamically?
>
> > So, your code can be removed.
>
> I don't think so. It does not break anything yet. One who make
> changes, should ensure backwards compatibility, at least talking about
> functionality and hardware (LCD) support, which was added few months
> ago. Remember, we talk about kernel parameters line - now it lets
> bootloader to select correct LCD size. After your changes, board with
> 7" LCD wont show anything.
[CC'ed Tushar Behera]
Yes, your code is working. However, your code is bug.
'struct s3c_fb_pd_win' is used not for LCD, but for windows of FIMD IP. So, 'struct s3c_fb_pd_win' should not be used to select LCD.
The mach-aquila.c uses two 'struct s3c_fb_pd_win' like you, however 'struct s3c_fb_pd_win' don't make problem with Thomas's
patchset. It is because mach-aquila.c use 'struct s3c_fb_pd_win' for only windows of FIMD IP.
We don't have to responsibility to keep wrong code. If you have kept the rule, Thomas's patch would not make the compatibility
problem with your code. Why we should keep aberration such as your code? If you want to select two LCDs, please find other way.
>
> Darius A.
^ permalink raw reply
* Re: [PATCH] x86: export 'pcibios_enabled'
From: Wang YanQing @ 2012-03-16 0:41 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Randy Dunlap, Stephen Rothwell, linux-next, LKML,
Michal Januszewski, Florian Tobias Schandinat, linux-fbdev, x86,
Andrew Morton
In-Reply-To: <4F5FE9AD.7000204@zytor.com>
On Tue, Mar 13, 2012 at 05:43:25PM -0700, H. Peter Anvin wrote:
> On 03/13/2012 01:30 PM, Randy Dunlap wrote:
> > From: Randy Dunlap <rdunlap@xenotime.net>
> >
> > Export 'pcibios_enabled' so that when uvesafb is built as a
> > loadable module (on X86_32), the build will succeed.
> >
> > ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> >
> > Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> > Cc: Michal Januszewski <spock@gentoo.org>
> > Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: x86@kernel.org
> > ---
> > Applies to mainline; found in linux-next.
> >
> > arch/x86/pci/pcbios.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> > +++ linux-next-20120313/arch/x86/pci/pcbios.c
> > @@ -27,6 +27,7 @@
> > #define PCIBIOS_HW_TYPE2_SPEC 0x20
> >
> > int pcibios_enabled;
> > +EXPORT_SYMBOL(pcibios_enabled);
> >
> > /* According to the BIOS specification at:
> > * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
>
> I would think this should be EXPORT_SYMBOL_GPL()... this seems like a
> symbol with a very high likelihood to be abused in strange ways.
>
> -hpa
>
Yes, I think EXPORT_SYMBOL_GPL is better. Indeed, there is another issue I
meet about the pcibios NX protection code.
If I set "pcibios_enable = 1" forcely no matter whether set_bios_x had been executed, then
the BIOS code is not NX. The problem is, if set_bios_x had not been executed, of course set_memory_x
for the bios code page has no chance to execute, then why the BIOS code is not NX? Any comment?
Or because the default set for the BIOS range is X, if so we should set_memory_nx if pcibios_enable = 0.
^ permalink raw reply
* Re: [PATCH 2/2] OMAPDSS: DISPC: Handle synclost errors in OMAP3
From: Ville Syrjälä @ 2012-03-15 23:04 UTC (permalink / raw)
To: Chandrabhanu Mahapatra; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <1331812132-9814-1-git-send-email-cmahapatra@ti.com>
On Thu, Mar 15, 2012 at 05:18:52PM +0530, Chandrabhanu Mahapatra wrote:
> In OMAP3 DISPC video overlays suffer from some undocumented horizontal position
> and timing related limitations leading to SYNCLOST errors. Whenever the image
> window is moved towards the right of the screen SYNCLOST errors become
> frequent. Checks have been implemented to see that DISPC driver rejects
> configuration exceeding above limitations.
>
> This code was successfully tested on OMAP3. This code is written based on code
> written by Ville Syrjälä <ville.syrjala@nokia.com> in Linux OMAP kernel. Ville
> Syrjälä <ville.syrjala@nokia.com> had added checks for video overlay horizontal
> timing and DISPC horizontal blanking length limitations.
>
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 67 +++++++++++++++++++++++++++++++++++++--
> 1 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 5a1963e..ebfa613 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -1622,6 +1622,58 @@ static void calc_dma_rotation_offset(u8 rotation, bool mirror,
> }
> }
>
> +static int check_horiz_timing(enum omap_channel channel, u16 pos_x,
> + u16 width, u16 height, u16 out_width, u16 out_height)
> +{
> + int DS = DIV_ROUND_UP(height, out_height);
> + struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
> + struct omap_video_timings t = dssdev->panel.timings;
> + int pcd = REG_GET(DISPC_DIVISORo(channel), 7, 0);
pcd doesn't exist for TV out, which is the reason why you see
'lclk/pclk' ratio used instead in the harmattan kernel.
Another thing which still seems to be wrong in the upstream code is the
use of fclk in the scaling checks. It should be checking lclk instead.
> + unsigned long nonactive, val, blank;
> + static const u8 limits[3] = { 8, 10, 20 };
> + int i;
> +
> + nonactive = t.x_res + t.hfp + t.hsw + t.hbp - out_width;
> +
> + /*
> + * Atleast DS-2 lines must have already been fetched
> + * before the display active video period starts.
> + */
> + val = (nonactive - pos_x) * pcd;
> + DSSDBG("(nonactive - pos_x) * pcd = %lu,"
> + " max(0, DS - 2) * width = %d\n",
> + val, max(0, DS - 2) * width);
> + if (val < max(0, DS - 2) * width)
> + return -EINVAL;
> +
> + /*
> + * Only one line can be fetched during the overlay active
> + * period, the rest have to be fetched during the inactive
> + * period.
> + */
> + val = nonactive * pcd;
> + DSSDBG("nonactive * pcd = %lu, max(0, DS - 1) * width = %d\n",
> + val, max(0, DS - 1) * width);
> + if (val < max(0, DS - 1) * width)
> + return -EINVAL;
> +
> + /*
> + * Atleast Ceil(DS) lines should have been loaded during
> + * PPL (screen width) + blanking period.
> + */
> + i = 0;
> + if (out_height < height)
> + i++;
> + if (out_width < width)
> + i++;
> + blank = (t.hbp + t.hsw + t.hfp) * pcd;
> + DSSDBG("blanking period + ppl = %lu (limit = %u)\n", blank, limits[i]);
> + if (blank <= limits[i])
> + return -EINVAL;
The comment and code here are totally out of sync. IIRC the hblank
length check came directly from the TRM. I have no idea if it's correct
for more recent OMAPs, or if it was just copy pasted from the
old TRM when the TRM got adjusted for more recent chips. That is why I
stuck it into a separate function in harmattan.
--
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/
^ permalink raw reply
* Re: [PATCH 1/2] OMAPDSS: DISPC: Enable predecimation
From: Ville Syrjälä @ 2012-03-15 22:28 UTC (permalink / raw)
To: Chandrabhanu Mahapatra; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <1331812108-9776-1-git-send-email-cmahapatra@ti.com>
On Thu, Mar 15, 2012 at 05:18:28PM +0530, Chandrabhanu Mahapatra wrote:
> In OMAP3 and OMAP4, the DISPC Scaler can downscale an image up to 4 times, and
> up to 2 times in OMAP2. However, with predecimation, the image can be reduced
> to 16 times by fetching only the necessary pixels in memory. Then this
> predecimated image can be downscaled further by the DISPC scaler.
Now, where does that number 16 come from? IIRC the hardware can skip
basically any number of pixels/rows. I certainly didn't add any such
limit to the code in the harmattan kernel, and distinctly remember
being able to downscale the N9/N950 UI even down to 1 pixel size :)
> Based on the downscaling required, a prior calculation of predecimation values
> for width and height of an image is done. Since, Predecimation reduces quality
> of an image higher priorty is given to DISPC Scaler for downscaling.
>
> This code was successfully tested on OMAP2, OMAP3 and OMAP4. Horizontal and
> vertical predecimation worked fine except for some synclost errors due to
> undocumented errata in OMAP3 which are fixed later and skewed images were seen
> on OMAP2 and OMAP3 during horizontal predecimation which will be addressed in
> the future patches.
All the rotation offset calculations still look suspiciously different
to what is in the harmattan kernel. I remember that the original code
was quite broken, and I fixed a lot of things when I was implementing
pre-decimation and some rotation stuff for the N9/N950. Too bad I never
managed to push that stuff upstream...
--
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/
^ permalink raw reply
* Re: [Linux-fbdev-devel] [uclinux-dist-devel] [PATCH] [#7001] framebuffer: old image of last app woul
From: Mike Frysinger @ 2012-03-15 19:29 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <201203151508.04564.vapier@gentoo.org>
[-- Attachment #1: Type: Text/Plain, Size: 896 bytes --]
On Thursday 15 March 2012 15:22:34 Geert Uytterhoeven wrote:
> On Thu, Mar 15, 2012 at 20:08, Mike Frysinger wrote:
> > On Thursday 15 March 2012 05:23:50 Wu, Aaron wrote:
> >> Old image of last application would retain for a while when starting a
> >> new application, this patch clear the frambuffer before displaying
> >> every time the fb is opened.
> >
> > i'm not sure the behavior you describe is wrong. in fact, i'm pretty
> > sure it sounds correct. if an app writes an image to the framebuffer
> > and then quits, that image should stay there indefinitely until
> > something else opens the framebuffer and draws their own image.
>
> Indeed. Else can you no longer make a screenshot using "cat /dev/fb0 >
> file".
ironically, that's one method we've documented for Blackfin platforms :)
http://docs.blackfin.uclinux.org/doku.php?id=uclinux-dist:framebuffer
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [Linux-fbdev-devel] [uclinux-dist-devel] [PATCH] [#7001] framebuffer: old image of last app woul
From: Geert Uytterhoeven @ 2012-03-15 19:22 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <201203151508.04564.vapier@gentoo.org>
[ Corrected obsolete linux-fbdev-devel address ]
On Thu, Mar 15, 2012 at 20:08, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 15 March 2012 05:23:50 Wu, Aaron wrote:
>> Old image of last application would retain for a while when starting a new
>> application, this patch clear the frambuffer before displaying every time
>> the fb is opened.
>
> i'm not sure the behavior you describe is wrong. in fact, i'm pretty sure it
> sounds correct. if an app writes an image to the framebuffer and then quits,
> that image should stay there indefinitely until something else opens the
> framebuffer and draws their own image.
Indeed. Else can you no longer make a screenshot using "cat /dev/fb0 > file".
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: [Linux-fbdev-devel] [uclinux-dist-devel] [PATCH] [#7001] framebuffer: old image of last app woul
From: Mike Frysinger @ 2012-03-15 19:08 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1.1: Type: Text/Plain, Size: 513 bytes --]
On Thursday 15 March 2012 05:23:50 Wu, Aaron wrote:
> Old image of last application would retain for a while when starting a new
> application, this patch clear the frambuffer before displaying every time
> the fb is opened.
i'm not sure the behavior you describe is wrong. in fact, i'm pretty sure it
sounds correct. if an app writes an image to the framebuffer and then quits,
that image should stay there indefinitely until something else opens the
framebuffer and draws their own image.
-mike
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 191 bytes --]
------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
[-- Attachment #3: Type: text/plain, Size: 182 bytes --]
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
^ 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