* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14 14:11 UTC (permalink / raw)
To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9Me0RX3PY7Uwf=VuaHzaPtoNY0q9gxFg9H-Q3N2giC2Q@mail.gmail.com>
On Wed, 2011-09-14 at 17:50 +0530, K, Mythri P wrote:
> Hi,
>
> On Wed, Sep 14, 2011 at 2:27 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2011-09-14 at 14:18 +0530, K, Mythri P wrote:
> >> On Wed, Sep 14, 2011 at 2:04 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > On Wed, 2011-09-14 at 13:57 +0530, K, Mythri P wrote:
> >> >> On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> > <snip>
> >
> >> > I don't understand this one. How could this be more dynamic? The
> >> > function checks the HPD bit, which (based on my observation) shows the
> >> > status whether a display is connected or not.
> >> There is a GPIO which detects the +3.3V on the line and detects the
> >> cable connect , there is also an interrupt based way.This is ideally
> >> called a Hot-plug detect event according to the spec in HDMI terms.
> >> But what you are saying here is that it is just a poll on the state?
> >
> > Yes, it's just for polling, but I don't quite see the difference. A
> > hot-plug event notifies when the display is connected or disconnected,
> > and detect() tells if a display is connected. They are all about the
> > same thing.
> >
> >> >> So I said if the purpose of this function is only to check for the HPD
> >> >> state bit it is fine.
> >> >
> >> > What does HPD bit tell us then?
> >>
> >> HPD state bit tells whether the cable is connected and whether EDID is
> >
> > This sounds like a good bit to test then. So is there something wrong
> > with using HPD? How does the GPIO differ from HPD bit?
> >
> >> ready to be read, But this is a static check that is done in this
> >> function.
> >
> > I don't understand what you mean with "static". The bit changes
> > dynamically according to the connect/disconnect state, and the bit is
> > checked dynamically when detect() is called.
> >
> Well ! Who would call the detect and why ? By Dynamic i meant when the
> cable is physically disconnected and connected there is detection
> logic which can be implemented either by GPIo/Interrupts.
> When you say the cable is connected , what happens in this case when
> the cable is connected to say monitor of one resolution and then
> plugged out and put to the other. Instead with dynamic method the
> based on the physical connect and disconnect the notification would be
> sent to any listener.
Ok, I see now what you mean.
Yes, you are right, detect() does not "know" if the monitor has changed
between polls, so both notification and polling are needed. I
implemented only polling as there's no HPD event mechanism yet in
omapdss, and also because this was simple and gives DRM basic ability to
detect a monitor.
Tomi
^ permalink raw reply
* Re: [PATCH] backlight: l4f00242t03: Use gpio_request_one to simplify
From: Fabio Estevam @ 2011-09-14 18:50 UTC (permalink / raw)
To: linux-fbdev
Andrew,
Would you take this one?
Thanks,
Fabio Estevam
On Fri, Aug 19, 2011 at 12:58 AM, Estevam Fabio-R49496
<r49496@freescale.com> wrote:
> Richard,
>
> Ping?
>
> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Friday, June 24, 2011 3:26 PM
> To: linux-fbdev@vger.kernel.org
> Cc: rpurdie@rpsys.net; Fabio Estevam; Estevam Fabio-R49496
> Subject: [PATCH] backlight: l4f00242t03: Use gpio_request_one to simplify error handling
>
> Using gpio_request_one can make the error handling simpler.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/video/backlight/l4f00242t03.c | 17 +++++------------
> 1 files changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
> index 98ad3e5..d6b0812 100644
> --- a/drivers/video/backlight/l4f00242t03.c
> +++ b/drivers/video/backlight/l4f00242t03.c
> @@ -178,29 +178,22 @@ static int __devinit l4f00242t03_probe(struct spi_device *spi)
>
> priv->spi = spi;
>
> - ret = gpio_request(pdata->reset_gpio, "lcd l4f00242t03 reset");
> + ret = gpio_request_one(pdata->reset_gpio, GPIOF_OUT_INIT_HIGH,
> + "lcd l4f00242t03 reset");
> if (ret) {
> dev_err(&spi->dev,
> "Unable to get the lcd l4f00242t03 reset gpio.\n");
> goto err;
> }
>
> - ret = gpio_direction_output(pdata->reset_gpio, 1);
> - if (ret)
> - goto err2;
> -
> - ret = gpio_request(pdata->data_enable_gpio,
> - "lcd l4f00242t03 data enable");
> + ret = gpio_request_one(pdata->data_enable_gpio, GPIOF_OUT_INIT_LOW,
> + "lcd l4f00242t03 data enable");
> if (ret) {
> dev_err(&spi->dev,
> "Unable to get the lcd l4f00242t03 data en gpio.\n");
> goto err2;
> }
> -
> - ret = gpio_direction_output(pdata->data_enable_gpio, 0);
> - if (ret)
> - goto err3;
> -
> +
> if (pdata->io_supply) {
> priv->io_reg = regulator_get(NULL, pdata->io_supply);
>
> --
> 1.7.1
>
>
>
>
^ permalink raw reply
* Re: [PATCH V4 RESEND 1/6] video: s3c-fb: Add S5P64X0 specific s3c_fb_driverdata
From: Florian Tobias Schandinat @ 2011-09-14 21:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1315591251-17071-1-git-send-email-ajaykumar.rs@samsung.com>
On 09/09/2011 06:00 PM, Ajay Kumar wrote:
> This patch:
> -- Adds s3c_fb_driverdata for S5P64X0, which supports 3 windows.
> -- Also, register "s5p64x0-fb" type driver_data.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Acked-by: Jingoo Han <jg1.han@samsung.com>
> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/s3c-fb.c | 27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 4aecf21..0fda252 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -1859,6 +1859,30 @@ static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
> },
> };
>
> +static struct s3c_fb_driverdata s3c_fb_data_s5p64x0 = {
> + .variant = {
> + .nr_windows = 3,
> + .vidtcon = VIDTCON0,
> + .wincon = WINCON(0),
> + .winmap = WINxMAP(0),
> + .keycon = WKEYCON,
> + .osd = VIDOSD_BASE,
> + .osd_stride = 16,
> + .buf_start = VIDW_BUF_START(0),
> + .buf_size = VIDW_BUF_SIZE(0),
> + .buf_end = VIDW_BUF_END(0),
> +
> + .palette = {
> + [0] = 0x2400,
> + [1] = 0x2800,
> + [2] = 0x2c00,
> + },
> + },
> + .win[0] = &s3c_fb_data_s5p_wins[0],
> + .win[1] = &s3c_fb_data_s5p_wins[1],
> + .win[2] = &s3c_fb_data_s5p_wins[2],
> +};
> +
> static struct platform_device_id s3c_fb_driver_ids[] = {
> {
> .name = "s3c-fb",
> @@ -1872,6 +1896,9 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> }, {
> .name = "s3c2443-fb",
> .driver_data = (unsigned long)&s3c_fb_data_s3c2443,
> + }, {
> + .name = "s5p64x0-fb",
> + .driver_data = (unsigned long)&s3c_fb_data_s5p64x0,
> },
> {},
> };
^ permalink raw reply
* Re: [PATCH] smscufx: reduce number of casts in ufx_raw_rect
From: Florian Tobias Schandinat @ 2011-09-14 21:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315388046-1175-1-git-send-email-steve.glendinning@smsc.com>
On 09/07/2011 09:34 AM, Steve Glendinning wrote:
> Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/smscufx.c | 20 ++++++++++----------
> 1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/video/smscufx.c b/drivers/video/smscufx.c
> index c6b86e7..44c8cab 100644
> --- a/drivers/video/smscufx.c
> +++ b/drivers/video/smscufx.c
> @@ -807,7 +807,7 @@ static int ufx_ops_mmap(struct fb_info *info, struct vm_area_struct *vma)
> return 0;
> }
>
> -static void ufx_raw_rect(struct ufx_data *dev, char *cmd, int x, int y,
> +static void ufx_raw_rect(struct ufx_data *dev, u16 *cmd, int x, int y,
> int width, int height)
> {
> size_t packed_line_len = ALIGN((width * 2), 4);
> @@ -821,27 +821,27 @@ static void ufx_raw_rect(struct ufx_data *dev, char *cmd, int x, int y,
> *((u32 *)&cmd[0]) = cpu_to_le32(0x01);
>
> /* length word */
> - *((u32 *)&cmd[4]) = cpu_to_le32(packed_rect_len + 16);
> + *((u32 *)&cmd[2]) = cpu_to_le32(packed_rect_len + 16);
>
> - *((u16 *)&cmd[8]) = cpu_to_le16(x);
> - *((u16 *)&cmd[10]) = cpu_to_le16(y);
> - *((u16 *)&cmd[12]) = cpu_to_le16(width);
> - *((u16 *)&cmd[14]) = cpu_to_le16(height);
> + cmd[4] = cpu_to_le16(x);
> + cmd[5] = cpu_to_le16(y);
> + cmd[6] = cpu_to_le16(width);
> + cmd[7] = cpu_to_le16(height);
>
> /* frame base address */
> - *((u32 *)&cmd[16]) = cpu_to_le32(0 & 0xffffff80);
> + *((u32 *)&cmd[8]) = cpu_to_le32(0);
>
> /* color mode and horizontal resolution */
> - *((u16 *)&cmd[20]) = cpu_to_le16(0x4000 | dev->info->var.xres);
> + cmd[10] = cpu_to_le16(0x4000 | dev->info->var.xres);
>
> /* vertical resolution */
> - *((u16 *)&cmd[22]) = cpu_to_le16(dev->info->var.yres);
> + cmd[11] = cpu_to_le16(dev->info->var.yres);
>
> /* packed data */
> for (line = 0; line < height; line++) {
> const int line_offset = dev->info->fix.line_length * (y + line);
> const int byte_offset = line_offset + (x * BPP);
> - memcpy(&cmd[24 + (packed_line_len * line)],
> + memcpy(&cmd[(24 + (packed_line_len * line)) / 2],
> (char *)dev->info->fix.smem_start + byte_offset, width * BPP);
> }
> }
^ permalink raw reply
* Re: [REPOST][PATCH 1/2] fbdev: fix indentation in modedb.c
From: Florian Tobias Schandinat @ 2011-09-14 21:33 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315929909-26311-1-git-send-email-timur@freescale.com>
On 09/13/2011 04:05 PM, Timur Tabi wrote:
> Fix the incorrect indentation in functions fb_try_mode() and fb_find_mode().
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
Applied this patch, although I do not like this sort of patches very much. Well,
I guess it's an improvement, are you doing some real work on this code?
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/modedb.c | 444 ++++++++++++++++++++++++------------------------
> 1 files changed, 225 insertions(+), 219 deletions(-)
>
> diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
> index cb175fe..a9a907c 100644
> --- a/drivers/video/modedb.c
> +++ b/drivers/video/modedb.c
> @@ -491,55 +491,56 @@ EXPORT_SYMBOL(vesa_modes);
> static int fb_try_mode(struct fb_var_screeninfo *var, struct fb_info *info,
> const struct fb_videomode *mode, unsigned int bpp)
> {
> - int err = 0;
> -
> - DPRINTK("Trying mode %s %dx%d-%d@%d\n", mode->name ? mode->name : "noname",
> - mode->xres, mode->yres, bpp, mode->refresh);
> - var->xres = mode->xres;
> - var->yres = mode->yres;
> - var->xres_virtual = mode->xres;
> - var->yres_virtual = mode->yres;
> - var->xoffset = 0;
> - var->yoffset = 0;
> - var->bits_per_pixel = bpp;
> - var->activate |= FB_ACTIVATE_TEST;
> - var->pixclock = mode->pixclock;
> - var->left_margin = mode->left_margin;
> - var->right_margin = mode->right_margin;
> - var->upper_margin = mode->upper_margin;
> - var->lower_margin = mode->lower_margin;
> - var->hsync_len = mode->hsync_len;
> - var->vsync_len = mode->vsync_len;
> - var->sync = mode->sync;
> - var->vmode = mode->vmode;
> - if (info->fbops->fb_check_var)
> - err = info->fbops->fb_check_var(var, info);
> - var->activate &= ~FB_ACTIVATE_TEST;
> - return err;
> + int err = 0;
> +
> + DPRINTK("Trying mode %s %dx%d-%d@%d\n",
> + mode->name ? mode->name : "noname",
> + mode->xres, mode->yres, bpp, mode->refresh);
> + var->xres = mode->xres;
> + var->yres = mode->yres;
> + var->xres_virtual = mode->xres;
> + var->yres_virtual = mode->yres;
> + var->xoffset = 0;
> + var->yoffset = 0;
> + var->bits_per_pixel = bpp;
> + var->activate |= FB_ACTIVATE_TEST;
> + var->pixclock = mode->pixclock;
> + var->left_margin = mode->left_margin;
> + var->right_margin = mode->right_margin;
> + var->upper_margin = mode->upper_margin;
> + var->lower_margin = mode->lower_margin;
> + var->hsync_len = mode->hsync_len;
> + var->vsync_len = mode->vsync_len;
> + var->sync = mode->sync;
> + var->vmode = mode->vmode;
> + if (info->fbops->fb_check_var)
> + err = info->fbops->fb_check_var(var, info);
> + var->activate &= ~FB_ACTIVATE_TEST;
> + return err;
> }
>
> /**
> - * fb_find_mode - finds a valid video mode
> - * @var: frame buffer user defined part of display
> - * @info: frame buffer info structure
> - * @mode_option: string video mode to find
> - * @db: video mode database
> - * @dbsize: size of @db
> - * @default_mode: default video mode to fall back to
> - * @default_bpp: default color depth in bits per pixel
> + * fb_find_mode - finds a valid video mode
> + * @var: frame buffer user defined part of display
> + * @info: frame buffer info structure
> + * @mode_option: string video mode to find
> + * @db: video mode database
> + * @dbsize: size of @db
> + * @default_mode: default video mode to fall back to
> + * @default_bpp: default color depth in bits per pixel
> *
> - * Finds a suitable video mode, starting with the specified mode
> - * in @mode_option with fallback to @default_mode. If
> - * @default_mode fails, all modes in the video mode database will
> - * be tried.
> + * Finds a suitable video mode, starting with the specified mode
> + * in @mode_option with fallback to @default_mode. If
> + * @default_mode fails, all modes in the video mode database will
> + * be tried.
> *
> - * Valid mode specifiers for @mode_option:
> + * Valid mode specifiers for @mode_option:
> *
> - * <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m] or
> - * <name>[-<bpp>][@<refresh>]
> + * <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m] or
> + * <name>[-<bpp>][@<refresh>]
> *
> - * with <xres>, <yres>, <bpp> and <refresh> decimal numbers and
> - * <name> a string.
> + * with <xres>, <yres>, <bpp> and <refresh> decimal numbers and
> + * <name> a string.
> *
> * If 'M' is present after yres (and before refresh/bpp if present),
> * the function will compute the timings using VESA(tm) Coordinated
> @@ -551,12 +552,12 @@ static int fb_try_mode(struct fb_var_screeninfo *var, struct fb_info *info,
> *
> * 1024x768MR-8@60m - Reduced blank with margins at 60Hz.
> *
> - * NOTE: The passed struct @var is _not_ cleared! This allows you
> - * to supply values for e.g. the grayscale and accel_flags fields.
> + * NOTE: The passed struct @var is _not_ cleared! This allows you
> + * to supply values for e.g. the grayscale and accel_flags fields.
> *
> - * Returns zero for failure, 1 if using specified @mode_option,
> - * 2 if using specified @mode_option with an ignored refresh rate,
> - * 3 if default mode is used, 4 if fall back to any valid mode.
> + * Returns zero for failure, 1 if using specified @mode_option,
> + * 2 if using specified @mode_option with an ignored refresh rate,
> + * 3 if default mode is used, 4 if fall back to any valid mode.
> *
> */
>
> @@ -566,198 +567,203 @@ int fb_find_mode(struct fb_var_screeninfo *var,
> const struct fb_videomode *default_mode,
> unsigned int default_bpp)
> {
> - int i;
> -
> - /* Set up defaults */
> - if (!db) {
> - db = modedb;
> - dbsize = ARRAY_SIZE(modedb);
> - }
> -
> - if (!default_mode)
> - default_mode = &db[0];
> -
> - if (!default_bpp)
> - default_bpp = 8;
> -
> - /* Did the user specify a video mode? */
> - if (!mode_option)
> - mode_option = fb_mode_option;
> - if (mode_option) {
> - const char *name = mode_option;
> - unsigned int namelen = strlen(name);
> - int res_specified = 0, bpp_specified = 0, refresh_specified = 0;
> - unsigned int xres = 0, yres = 0, bpp = default_bpp, refresh = 0;
> - int yres_specified = 0, cvt = 0, rb = 0, interlace = 0, margins = 0;
> - u32 best, diff, tdiff;
> -
> - for (i = namelen-1; i >= 0; i--) {
> - switch (name[i]) {
> - case '@':
> - namelen = i;
> - if (!refresh_specified && !bpp_specified &&
> - !yres_specified) {
> - refresh = simple_strtol(&name[i+1], NULL, 10);
> - refresh_specified = 1;
> - if (cvt || rb)
> - cvt = 0;
> - } else
> - goto done;
> - break;
> - case '-':
> - namelen = i;
> - if (!bpp_specified && !yres_specified) {
> - bpp = simple_strtol(&name[i+1], NULL, 10);
> - bpp_specified = 1;
> - if (cvt || rb)
> - cvt = 0;
> - } else
> - goto done;
> - break;
> - case 'x':
> - if (!yres_specified) {
> - yres = simple_strtol(&name[i+1], NULL, 10);
> - yres_specified = 1;
> - } else
> - goto done;
> - break;
> - case '0' ... '9':
> - break;
> - case 'M':
> - if (!yres_specified)
> - cvt = 1;
> - break;
> - case 'R':
> - if (!cvt)
> - rb = 1;
> - break;
> - case 'm':
> - if (!cvt)
> - margins = 1;
> - break;
> - case 'i':
> - if (!cvt)
> - interlace = 1;
> - break;
> - default:
> - goto done;
> - }
> - }
> - if (i < 0 && yres_specified) {
> - xres = simple_strtol(name, NULL, 10);
> - res_specified = 1;
> - }
> -done:
> - if (cvt) {
> - struct fb_videomode cvt_mode;
> - int ret;
> -
> - DPRINTK("CVT mode %dx%d@%dHz%s%s%s\n", xres, yres,
> - (refresh) ? refresh : 60, (rb) ? " reduced blanking" :
> - "", (margins) ? " with margins" : "", (interlace) ?
> - " interlaced" : "");
> -
> - memset(&cvt_mode, 0, sizeof(cvt_mode));
> - cvt_mode.xres = xres;
> - cvt_mode.yres = yres;
> - cvt_mode.refresh = (refresh) ? refresh : 60;
> + int i;
>
> - if (interlace)
> - cvt_mode.vmode |= FB_VMODE_INTERLACED;
> - else
> - cvt_mode.vmode &= ~FB_VMODE_INTERLACED;
> + /* Set up defaults */
> + if (!db) {
> + db = modedb;
> + dbsize = ARRAY_SIZE(modedb);
> + }
>
> - ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
> + if (!default_mode)
> + default_mode = &db[0];
> +
> + if (!default_bpp)
> + default_bpp = 8;
> +
> + /* Did the user specify a video mode? */
> + if (!mode_option)
> + mode_option = fb_mode_option;
> + if (mode_option) {
> + const char *name = mode_option;
> + unsigned int namelen = strlen(name);
> + int res_specified = 0, bpp_specified = 0, refresh_specified = 0;
> + unsigned int xres = 0, yres = 0, bpp = default_bpp, refresh = 0;
> + int yres_specified = 0, cvt = 0, rb = 0, interlace = 0;
> + int margins = 0;
> + u32 best, diff, tdiff;
> +
> + for (i = namelen-1; i >= 0; i--) {
> + switch (name[i]) {
> + case '@':
> + namelen = i;
> + if (!refresh_specified && !bpp_specified &&
> + !yres_specified) {
> + refresh = simple_strtol(&name[i+1], NULL,
> + 10);
> + refresh_specified = 1;
> + if (cvt || rb)
> + cvt = 0;
> + } else
> + goto done;
> + break;
> + case '-':
> + namelen = i;
> + if (!bpp_specified && !yres_specified) {
> + bpp = simple_strtol(&name[i+1], NULL,
> + 10);
> + bpp_specified = 1;
> + if (cvt || rb)
> + cvt = 0;
> + } else
> + goto done;
> + break;
> + case 'x':
> + if (!yres_specified) {
> + yres = simple_strtol(&name[i+1], NULL,
> + 10);
> + yres_specified = 1;
> + } else
> + goto done;
> + break;
> + case '0' ... '9':
> + break;
> + case 'M':
> + if (!yres_specified)
> + cvt = 1;
> + break;
> + case 'R':
> + if (!cvt)
> + rb = 1;
> + break;
> + case 'm':
> + if (!cvt)
> + margins = 1;
> + break;
> + case 'i':
> + if (!cvt)
> + interlace = 1;
> + break;
> + default:
> + goto done;
> + }
> + }
> + if (i < 0 && yres_specified) {
> + xres = simple_strtol(name, NULL, 10);
> + res_specified = 1;
> + }
> +done:
> + if (cvt) {
> + struct fb_videomode cvt_mode;
> + int ret;
> +
> + DPRINTK("CVT mode %dx%d@%dHz%s%s%s\n", xres, yres,
> + (refresh) ? refresh : 60,
> + (rb) ? " reduced blanking" : "",
> + (margins) ? " with margins" : "",
> + (interlace) ? " interlaced" : "");
> +
> + memset(&cvt_mode, 0, sizeof(cvt_mode));
> + cvt_mode.xres = xres;
> + cvt_mode.yres = yres;
> + cvt_mode.refresh = (refresh) ? refresh : 60;
> +
> + if (interlace)
> + cvt_mode.vmode |= FB_VMODE_INTERLACED;
> + else
> + cvt_mode.vmode &= ~FB_VMODE_INTERLACED;
> +
> + ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
> +
> + if (!ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
> + DPRINTK("modedb CVT: CVT mode ok\n");
> + return 1;
> + }
>
> - if (!ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
> - DPRINTK("modedb CVT: CVT mode ok\n");
> - return 1;
> - }
> + DPRINTK("CVT mode invalid, getting mode from database\n");
> + }
>
> - DPRINTK("CVT mode invalid, getting mode from database\n");
> - }
> + DPRINTK("Trying specified video mode%s %ix%i\n",
> + refresh_specified ? "" : " (ignoring refresh rate)",
> + xres, yres);
>
> - DPRINTK("Trying specified video mode%s %ix%i\n",
> - refresh_specified ? "" : " (ignoring refresh rate)", xres, yres);
> -
> - if (!refresh_specified) {
> - /*
> - * If the caller has provided a custom mode database and a
> - * valid monspecs structure, we look for the mode with the
> - * highest refresh rate. Otherwise we play it safe it and
> - * try to find a mode with a refresh rate closest to the
> - * standard 60 Hz.
> - */
> - if (db != modedb &&
> - info->monspecs.vfmin && info->monspecs.vfmax &&
> - info->monspecs.hfmin && info->monspecs.hfmax &&
> - info->monspecs.dclkmax) {
> - refresh = 1000;
> - } else {
> - refresh = 60;
> + if (!refresh_specified) {
> + /*
> + * If the caller has provided a custom mode database and
> + * a valid monspecs structure, we look for the mode with
> + * the highest refresh rate. Otherwise we play it safe
> + * it and try to find a mode with a refresh rate closest
> + * to the standard 60 Hz.
> + */
> + if (db != modedb &&
> + info->monspecs.vfmin && info->monspecs.vfmax &&
> + info->monspecs.hfmin && info->monspecs.hfmax &&
> + info->monspecs.dclkmax) {
> + refresh = 1000;
> + } else {
> + refresh = 60;
> + }
> }
> - }
>
> - diff = -1;
> - best = -1;
> - for (i = 0; i < dbsize; i++) {
> - if ((name_matches(db[i], name, namelen) ||
> - (res_specified && res_matches(db[i], xres, yres))) &&
> - !fb_try_mode(var, info, &db[i], bpp)) {
> - if (refresh_specified && db[i].refresh = refresh) {
> - return 1;
> - } else {
> + diff = -1;
> + best = -1;
> + for (i = 0; i < dbsize; i++) {
> + if ((name_matches(db[i], name, namelen) ||
> + (res_specified && res_matches(db[i], xres, yres))) &&
> + !fb_try_mode(var, info, &db[i], bpp)) {
> + if (refresh_specified && db[i].refresh = refresh)
> + return 1;
> +
> if (abs(db[i].refresh - refresh) < diff) {
> diff = abs(db[i].refresh - refresh);
> best = i;
> }
> }
> }
> - }
> - if (best != -1) {
> - fb_try_mode(var, info, &db[best], bpp);
> - return (refresh_specified) ? 2 : 1;
> - }
> -
> - diff = 2 * (xres + yres);
> - best = -1;
> - DPRINTK("Trying best-fit modes\n");
> - for (i = 0; i < dbsize; i++) {
> - DPRINTK("Trying %ix%i\n", db[i].xres, db[i].yres);
> - if (!fb_try_mode(var, info, &db[i], bpp)) {
> - tdiff = abs(db[i].xres - xres) +
> - abs(db[i].yres - yres);
> -
> - /*
> - * Penalize modes with resolutions smaller
> - * than requested.
> - */
> - if (xres > db[i].xres || yres > db[i].yres)
> - tdiff += xres + yres;
> + if (best != -1) {
> + fb_try_mode(var, info, &db[best], bpp);
> + return (refresh_specified) ? 2 : 1;
> + }
>
> - if (diff > tdiff) {
> - diff = tdiff;
> - best = i;
> + diff = 2 * (xres + yres);
> + best = -1;
> + DPRINTK("Trying best-fit modes\n");
> + for (i = 0; i < dbsize; i++) {
> + DPRINTK("Trying %ix%i\n", db[i].xres, db[i].yres);
> + if (!fb_try_mode(var, info, &db[i], bpp)) {
> + tdiff = abs(db[i].xres - xres) +
> + abs(db[i].yres - yres);
> +
> + /*
> + * Penalize modes with resolutions smaller
> + * than requested.
> + */
> + if (xres > db[i].xres || yres > db[i].yres)
> + tdiff += xres + yres;
> +
> + if (diff > tdiff) {
> + diff = tdiff;
> + best = i;
> + }
> }
> }
> + if (best != -1) {
> + fb_try_mode(var, info, &db[best], bpp);
> + return 5;
> + }
> }
> - if (best != -1) {
> - fb_try_mode(var, info, &db[best], bpp);
> - return 5;
> - }
> - }
>
> - DPRINTK("Trying default video mode\n");
> - if (!fb_try_mode(var, info, default_mode, default_bpp))
> - return 3;
> + DPRINTK("Trying default video mode\n");
> + if (!fb_try_mode(var, info, default_mode, default_bpp))
> + return 3;
>
> - DPRINTK("Trying all modes\n");
> - for (i = 0; i < dbsize; i++)
> - if (!fb_try_mode(var, info, &db[i], default_bpp))
> - return 4;
> + DPRINTK("Trying all modes\n");
> + for (i = 0; i < dbsize; i++)
> + if (!fb_try_mode(var, info, &db[i], default_bpp))
> + return 4;
>
> - DPRINTK("No valid mode found\n");
> - return 0;
> + DPRINTK("No valid mode found\n");
> + return 0;
> }
>
> /**
^ permalink raw reply
* Re: [REPOST][PATCH 1/2] fbdev: fix indentation in modedb.c
From: Timur Tabi @ 2011-09-14 21:39 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315929909-26311-1-git-send-email-timur@freescale.com>
Florian Tobias Schandinat wrote:
> Applied this patch, although I do not like this sort of patches very much. Well,
> I guess it's an improvement, are you doing some real work on this code?
Yes, but I needed to make it sane first. The big issue is that it initializes
the hardware when the driver loads, not when it's open by the framebuffer layer.
I understand the complaint against multiple changes combined into one patch, but
all I was doing was just going through the code line-by-line and looking for
minor problems.
I do appreciate your applying the patch as-is. I was not looking forward to
breaking it up into multiple pieces.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [REPOST][PATCH 1/2] fbdev: fix indentation in modedb.c
From: Timur Tabi @ 2011-09-14 21:47 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315929909-26311-1-git-send-email-timur@freescale.com>
Florian Tobias Schandinat wrote:
> Applied this patch, although I do not like this sort of patches very much. Well,
> I guess it's an improvement, are you doing some real work on this code?
Ugh, please ignore my previous reply. For some reason, I thought you were
talking about my second patch.
To answer your question properly: no, I'm not doing any real work on this code.
I noticed the bad indentation, and I decided that maybe it was worth fixing.
Yes, it might cause some pain with diffs, but these functions have been around a
long time and haven't been touched in while. Also, a similar patch to mine was
applied recently: "video: tidy up modedb formatting."
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [REPOST][PATCH 1/2] fbdev: fix indentation in modedb.c
From: Florian Tobias Schandinat @ 2011-09-14 22:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315929909-26311-1-git-send-email-timur@freescale.com>
On 09/14/2011 09:47 PM, Timur Tabi wrote:
> Florian Tobias Schandinat wrote:
>> Applied this patch, although I do not like this sort of patches very much. Well,
>> I guess it's an improvement, are you doing some real work on this code?
>
> Ugh, please ignore my previous reply. For some reason, I thought you were
> talking about my second patch.
No problem. Are you going to answer Tormod's email?
I agree with him, that those should have been separate patches and I wasn't sure
whether to reject it or accept it one last time (future patches with such a list
in the commit message will be rejected for sure). I guess I might let it slip
through this time but at least answer to the type of edid.
> To answer your question properly: no, I'm not doing any real work on this code.
> I noticed the bad indentation, and I decided that maybe it was worth fixing.
> Yes, it might cause some pain with diffs, but these functions have been around a
> long time and haven't been touched in while. Also, a similar patch to mine was
> applied recently: "video: tidy up modedb formatting."
Okay, I admit that it might have improved readability and by using "diff -b"
most of it was easy reviewable. The thing that really costs me some time was
checking the braces after your removal of
else {
after an if-statement that included a return.
Such patches are not always bad, but they have less priority than real changes
and I really wouldn't recommend anyone to try "fixing" any checkpatch warning
inside the kernel source.
Best regards,
Florian Tobias Schandinat
^ permalink raw reply
* Re: [REPOST][PATCH 1/2] fbdev: fix indentation in modedb.c
From: Tabi Timur-B04825 @ 2011-09-15 1:39 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315929909-26311-1-git-send-email-timur@freescale.com>
Florian Tobias Schandinat wrote:
> On 09/14/2011 09:47 PM, Timur Tabi wrote:
>> Florian Tobias Schandinat wrote:
>>> Applied this patch, although I do not like this sort of patches very much. Well,
>>> I guess it's an improvement, are you doing some real work on this code?
>>
>> Ugh, please ignore my previous reply. For some reason, I thought you were
>> talking about my second patch.
>
> No problem. Are you going to answer Tormod's email?
Yes, definitely.
> Such patches are not always bad, but they have less priority than real changes
> and I really wouldn't recommend anyone to try "fixing" any checkpatch warning
> inside the kernel source.
I wouldn't have posted it if I thought that it would cause merge problems.
Like I said, those two functions haven't seen any activity in a long time.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-15 5:53 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1316009509.2118.6.camel@deskari>
Hi,
On Wed, Sep 14, 2011 at 7:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2011-09-14 at 17:50 +0530, K, Mythri P wrote:
>> Hi,
>>
>> On Wed, Sep 14, 2011 at 2:27 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > On Wed, 2011-09-14 at 14:18 +0530, K, Mythri P wrote:
>> >> On Wed, Sep 14, 2011 at 2:04 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> >> > On Wed, 2011-09-14 at 13:57 +0530, K, Mythri P wrote:
>> >> >> On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> >
>> > <snip>
>> >
>> >> > I don't understand this one. How could this be more dynamic? The
>> >> > function checks the HPD bit, which (based on my observation) shows the
>> >> > status whether a display is connected or not.
>> >> There is a GPIO which detects the +3.3V on the line and detects the
>> >> cable connect , there is also an interrupt based way.This is ideally
>> >> called a Hot-plug detect event according to the spec in HDMI terms.
>> >> But what you are saying here is that it is just a poll on the state?
>> >
>> > Yes, it's just for polling, but I don't quite see the difference. A
>> > hot-plug event notifies when the display is connected or disconnected,
>> > and detect() tells if a display is connected. They are all about the
>> > same thing.
>> >
>> >> >> So I said if the purpose of this function is only to check for the HPD
>> >> >> state bit it is fine.
>> >> >
>> >> > What does HPD bit tell us then?
>> >>
>> >> HPD state bit tells whether the cable is connected and whether EDID is
>> >
>> > This sounds like a good bit to test then. So is there something wrong
>> > with using HPD? How does the GPIO differ from HPD bit?
>> >
>> >> ready to be read, But this is a static check that is done in this
>> >> function.
>> >
>> > I don't understand what you mean with "static". The bit changes
>> > dynamically according to the connect/disconnect state, and the bit is
>> > checked dynamically when detect() is called.
>> >
>> Well ! Who would call the detect and why ? By Dynamic i meant when the
>> cable is physically disconnected and connected there is detection
>> logic which can be implemented either by GPIo/Interrupts.
>> When you say the cable is connected , what happens in this case when
>> the cable is connected to say monitor of one resolution and then
>> plugged out and put to the other. Instead with dynamic method the
>> based on the physical connect and disconnect the notification would be
>> sent to any listener.
>
> Ok, I see now what you mean.
>
> Yes, you are right, detect() does not "know" if the monitor has changed
> between polls, so both notification and polling are needed. I
> implemented only polling as there's no HPD event mechanism yet in
> omapdss, and also because this was simple and gives DRM basic ability to
> detect a monitor.
>
If it is needed for DRM then it is fine, but with detect renamed to
poll. By next week i should have a patch ready for HPD event
mechanism.
> Tomi
>
>
>
Thanks and regards,
Mythri.
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-15 5:57 UTC (permalink / raw)
To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B-_19gPDmjJGBHpcDcGPGyt9QvFXFwgjGPUqr-QY9SPJg@mail.gmail.com>
On Thu, 2011-09-15 at 11:11 +0530, K, Mythri P wrote:
> Hi,
>
> On Wed, Sep 14, 2011 at 7:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > Yes, you are right, detect() does not "know" if the monitor has changed
> > between polls, so both notification and polling are needed. I
> > implemented only polling as there's no HPD event mechanism yet in
> > omapdss, and also because this was simple and gives DRM basic ability to
> > detect a monitor.
> >
> If it is needed for DRM then it is fine, but with detect renamed to
> poll. By next week i should have a patch ready for HPD event
> mechanism.
What is wrong with "detect"? It detects if there's a display connected.
It can be used in polling manner, trying it every n seconds, but it
should also be used even if you use HPD event. I think the normal
sequence would be something like:
1) register HPD event
2) use detect() to see if a monitor is already connected
Tomi
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-15 6:32 UTC (permalink / raw)
To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B_WvtNTi9ckyor-+5bo7n6GkeTSAvhDsrGQxhPZWgXO0w@mail.gmail.com>
On Thu, 2011-09-15 at 11:54 +0530, K, Mythri P wrote:
> Hi,
>
> On Thu, Sep 15, 2011 at 11:27 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Thu, 2011-09-15 at 11:11 +0530, K, Mythri P wrote:
> >> Hi,
> >>
> >> On Wed, Sep 14, 2011 at 7:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> >> > Yes, you are right, detect() does not "know" if the monitor has changed
> >> > between polls, so both notification and polling are needed. I
> >> > implemented only polling as there's no HPD event mechanism yet in
> >> > omapdss, and also because this was simple and gives DRM basic ability to
> >> > detect a monitor.
> >> >
> >> If it is needed for DRM then it is fine, but with detect renamed to
> >> poll. By next week i should have a patch ready for HPD event
> >> mechanism.
> >
> > What is wrong with "detect"? It detects if there's a display connected.
> > It can be used in polling manner, trying it every n seconds, but it
> > should also be used even if you use HPD event. I think the normal
> > sequence would be something like:
> >
> > 1) register HPD event
> > 2) use detect() to see if a monitor is already connected
> >
> I guess polling ever few seconds to detect would be waste of CPU
> cycles when there is already a mechanism in the H/w to detect the
> connection.
Obviously. Polling is only used if hot-plug-detect is not available. But
detect function can be used even when HPD is available.
> Current sequence :
> Enable display ( Irrespective of whether the cable is connected on not)
>
> Sequence with HPD:
> 1.Register for HPD connect.
> 2.Enable display
> 3.Notify DRM/Audio/Kernel component that wants to listen to this event.
Why would you enable the display even if there's no monitor connected?
And when the DRM starts, how does DRM know if the display was already
connected? Would you send a HPD event when DRM registers to the event
even if there's no actual plug-in event done (i.e. user actually
connecting the cable)?
And just to clarify, my sequence example was from DRM's point of view.
The HDMI driver shouldn't do anything before DRM/omapfb asks it to do
something.
Tomi
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-15 6:36 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1316066265.1880.6.camel@deskari>
Hi,
On Thu, Sep 15, 2011 at 11:27 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2011-09-15 at 11:11 +0530, K, Mythri P wrote:
>> Hi,
>>
>> On Wed, Sep 14, 2011 at 7:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>> > Yes, you are right, detect() does not "know" if the monitor has changed
>> > between polls, so both notification and polling are needed. I
>> > implemented only polling as there's no HPD event mechanism yet in
>> > omapdss, and also because this was simple and gives DRM basic ability to
>> > detect a monitor.
>> >
>> If it is needed for DRM then it is fine, but with detect renamed to
>> poll. By next week i should have a patch ready for HPD event
>> mechanism.
>
> What is wrong with "detect"? It detects if there's a display connected.
> It can be used in polling manner, trying it every n seconds, but it
> should also be used even if you use HPD event. I think the normal
> sequence would be something like:
>
> 1) register HPD event
> 2) use detect() to see if a monitor is already connected
>
I guess polling ever few seconds to detect would be waste of CPU
cycles when there is already a mechanism in the H/w to detect the
connection.
Current sequence :
Enable display ( Irrespective of whether the cable is connected on not)
Sequence with HPD:
1.Register for HPD connect.
2.Enable display
3.Notify DRM/Audio/Kernel component that wants to listen to this event.
Thanks and regards,
Mythri.
^ permalink raw reply
* How to use backlight device with fb device
From: Tomi Valkeinen @ 2011-09-15 7:47 UTC (permalink / raw)
To: linux-fbdev
Hi,
I'm a bit confused how a backlight device and a fb device should be
"connected".
In this particular case the OMAP 4430SDP board has an LCD panel
controlled by omapfb (and omapdss on the lower level) with a backlight
controlled by pwm_backlight (and twl6030-pwm on the lower level).
Both omapfb and pwm_backlight work fine, but they don't know anything
about each other, so, for example, blanking the fb via omapfb leaves the
backlight on.
Is there something I'm missing, or is the only solution to have some
custom code to connect the fb and the bl, and handle it that way?
Tomi
^ permalink raw reply
* Re: How to use backlight device with fb device
From: Florian Tobias Schandinat @ 2011-09-15 8:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1316072820.1880.51.camel@deskari>
Hi Tomi,
On 09/15/2011 07:47 AM, Tomi Valkeinen wrote:
> Hi,
>
> I'm a bit confused how a backlight device and a fb device should be
> "connected".
>
> In this particular case the OMAP 4430SDP board has an LCD panel
> controlled by omapfb (and omapdss on the lower level) with a backlight
> controlled by pwm_backlight (and twl6030-pwm on the lower level).
>
> Both omapfb and pwm_backlight work fine, but they don't know anything
> about each other, so, for example, blanking the fb via omapfb leaves the
> backlight on.
>
> Is there something I'm missing, or is the only solution to have some
> custom code to connect the fb and the bl, and handle it that way?
I don't know anything about backlight but recently there was a patch that
affected the same area you mention so it might give an idea
"[PATCH] FB: add early fb blank feature."
http://marc.info/?l=linux-fbdev&m\x131554458720940&w=2
Best regards,
Florian Tobias Schandinat
>
> Tomi
>
>
> --
> 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: How to use backlight device with fb device
From: Tomi Valkeinen @ 2011-09-15 9:18 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1316072820.1880.51.camel@deskari>
On Thu, 2011-09-15 at 08:58 +0000, Florian Tobias Schandinat wrote:
> Hi Tomi,
>
> On 09/15/2011 07:47 AM, Tomi Valkeinen wrote:
> > Hi,
> >
> > I'm a bit confused how a backlight device and a fb device should be
> > "connected".
> >
> > In this particular case the OMAP 4430SDP board has an LCD panel
> > controlled by omapfb (and omapdss on the lower level) with a backlight
> > controlled by pwm_backlight (and twl6030-pwm on the lower level).
> >
> > Both omapfb and pwm_backlight work fine, but they don't know anything
> > about each other, so, for example, blanking the fb via omapfb leaves the
> > backlight on.
> >
> > Is there something I'm missing, or is the only solution to have some
> > custom code to connect the fb and the bl, and handle it that way?
>
> I don't know anything about backlight but recently there was a patch that
> affected the same area you mention so it might give an idea
> "[PATCH] FB: add early fb blank feature."
> http://marc.info/?l=linux-fbdev&m\x131554458720940&w=2
Ah, thanks for pointing that out.
I seem to have dropped from linux-fbdev list and hadn't received that
post. For some reason I seem to get dropped from linux mailing lists,
this is getting a bit frustrating...
Tomi
^ permalink raw reply
* Re: [PATCH] FB: add early fb blank feature.
From: Tomi Valkeinen @ 2011-09-15 9:53 UTC (permalink / raw)
To: Inki Dae
Cc: FlorianSchandinat, linux-fbdev, akpm, linux-kernel, kyungmin.park
In-Reply-To: <1315544581-16379-1-git-send-email-inki.dae@samsung.com>
Hi,
On Fri, 2011-09-09 at 14:03 +0900, Inki Dae wrote:
> this patch adds early fb blank feature that this is a callback of
> lcd panel driver would be called prior to fb driver's one.
> in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> the power off commands should be transferred to lcd panel with display
> and mipi-dsi controller enabled because the commands is set to lcd panel
> at vsync porch period. on the other hand, in opposite case, the callback
> of fb driver should be called prior to lcd panel driver's one because of
> same issue. now we could handle call order to fb blank properly.
>
> the order is as the following:
>
> at fb_blank function of fbmem.c
> -> fb_early_notifier_call_chain()
> -> lcd panel driver's early_set_power()
> -> info->fbops->fb_blank()
> -> fb driver's fb_blank()
> -> fb_notifier_call_chain()
> -> lcd panel driver's set_power()
I'm not familiar with the lcd.c, so I may be talking nonsense, but I
don't quite understand the need for this patch. If you have some kind of
panel driver, shouldn't the panel driver handle power off in just one
place?
With omapfb and omapdss, the omapfb's fb_blank function just calls power
off in the panel driver, which handles all necessary actions. Is your
model somehow totally different?
Tomi
^ permalink raw reply
* RE: [PATCH] FB: add early fb blank feature.
From: Inki Dae @ 2011-09-15 10:14 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315544581-16379-1-git-send-email-inki.dae@samsung.com>
Hi, Tomi.
> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@ti.com]
> Sent: Thursday, September 15, 2011 6:53 PM
> To: Inki Dae
> Cc: FlorianSchandinat@gmx.de; linux-fbdev@vger.kernel.org; akpm@linux-
> foundation.org; linux-kernel@vger.kernel.org; kyungmin.park@samsung.com
> Subject: Re: [PATCH] FB: add early fb blank feature.
>
> Hi,
>
> On Fri, 2011-09-09 at 14:03 +0900, Inki Dae wrote:
> > this patch adds early fb blank feature that this is a callback of
> > lcd panel driver would be called prior to fb driver's one.
> > in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> > the power off commands should be transferred to lcd panel with display
> > and mipi-dsi controller enabled because the commands is set to lcd panel
> > at vsync porch period. on the other hand, in opposite case, the callback
> > of fb driver should be called prior to lcd panel driver's one because of
> > same issue. now we could handle call order to fb blank properly.
> >
> > the order is as the following:
> >
> > at fb_blank function of fbmem.c
> > -> fb_early_notifier_call_chain()
> > -> lcd panel driver's early_set_power()
> > -> info->fbops->fb_blank()
> > -> fb driver's fb_blank()
> > -> fb_notifier_call_chain()
> > -> lcd panel driver's set_power()
>
> I'm not familiar with the lcd.c, so I may be talking nonsense, but I
> don't quite understand the need for this patch. If you have some kind of
> panel driver, shouldn't the panel driver handle power off in just one
> place?
>
Yes, almost lcd panels are ok. but for command setting, our lcd panel, mipi-dsi based RGB panel, should be power on before it transfers commands to panel. for example, if user requested FB_BLANK_POWERDOWN then first, display controller would be off and then set_power callback of lcd panel driver would be called. at this time, it has a problem. The problem is that display controller already is off so lcd panel can't accept some commands, such as sleep in command. Lcd panel can accept such commands with vsync period. And also there is another case. This is sparkling issue and would have implications for all ones.
for this, you can refer to a link below:
< http://adras.com/fixed-sparkling-issue-on-lcd-panel-when-fb-blank-mode-is.t196691-141.html >
> With omapfb and omapdss, the omapfb's fb_blank function just calls power
> off in the panel driver, which handles all necessary actions. Is your
> model somehow totally different?
>
> Tomi
Best Regards,
Inki Dae.
^ permalink raw reply
* RE: [PATCH] FB: add early fb blank feature.
From: Tomi Valkeinen @ 2011-09-15 10:26 UTC (permalink / raw)
To: Inki Dae
Cc: FlorianSchandinat, linux-fbdev, akpm, linux-kernel, kyungmin.park
In-Reply-To: <000001cc7390$482c00f0$d88402d0$%dae@samsung.com>
On Thu, 2011-09-15 at 19:14 +0900, Inki Dae wrote:
> Hi, Tomi.
>
> > -----Original Message-----
> > From: Tomi Valkeinen [mailto:tomi.valkeinen@ti.com]
> > Sent: Thursday, September 15, 2011 6:53 PM
> > To: Inki Dae
> > Cc: FlorianSchandinat@gmx.de; linux-fbdev@vger.kernel.org; akpm@linux-
> > foundation.org; linux-kernel@vger.kernel.org; kyungmin.park@samsung.com
> > Subject: Re: [PATCH] FB: add early fb blank feature.
> >
> > Hi,
> >
> > On Fri, 2011-09-09 at 14:03 +0900, Inki Dae wrote:
> > > this patch adds early fb blank feature that this is a callback of
> > > lcd panel driver would be called prior to fb driver's one.
> > > in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> > > the power off commands should be transferred to lcd panel with display
> > > and mipi-dsi controller enabled because the commands is set to lcd panel
> > > at vsync porch period. on the other hand, in opposite case, the callback
> > > of fb driver should be called prior to lcd panel driver's one because of
> > > same issue. now we could handle call order to fb blank properly.
> > >
> > > the order is as the following:
> > >
> > > at fb_blank function of fbmem.c
> > > -> fb_early_notifier_call_chain()
> > > -> lcd panel driver's early_set_power()
> > > -> info->fbops->fb_blank()
> > > -> fb driver's fb_blank()
> > > -> fb_notifier_call_chain()
> > > -> lcd panel driver's set_power()
> >
> > I'm not familiar with the lcd.c, so I may be talking nonsense, but I
> > don't quite understand the need for this patch. If you have some kind of
> > panel driver, shouldn't the panel driver handle power off in just one
> > place?
> >
>
>
> Yes, almost lcd panels are ok. but for command setting, our lcd panel, mipi-dsi based RGB panel, should be power on before it transfers commands to panel. for example, if user requested FB_BLANK_POWERDOWN then first, display controller would be off and then set_power callback of lcd panel driver would be called. at this time, it has a problem. The problem is that display controller already is off so lcd panel can't accept some commands, such as sleep in command. Lcd panel can accept such commands with vsync period. And also there is another case. This is sparkling issue and would have implications for all ones.
> for this, you can refer to a link below:
> < http://adras.com/fixed-sparkling-issue-on-lcd-panel-when-fb-blank-mode-is.t196691-141.html >
Ok, I see. The architecture for omap display is a bit different, so we
don't have similar problems. We don't use the fb notifier at all, but
omapfb handles calling the necessary functions in the panel driver.
Tomi
^ permalink raw reply
* Re: [PATCH] FB: add early fb blank feature.
From: Lars-Peter Clausen @ 2011-09-15 11:37 UTC (permalink / raw)
To: Inki Dae
Cc: FlorianSchandinat, linux-fbdev, akpm, linux-kernel, kyungmin.park
In-Reply-To: <1315544581-16379-1-git-send-email-inki.dae@samsung.com>
Hi
I have a LCD panel with an similar issue, and I think the idea to introduce a
early fb blank event is the right solution. I have some comments and questions
on this particular implementation though.
On 09/09/2011 07:03 AM, Inki Dae wrote:
> this patch adds early fb blank feature that this is a callback of
> lcd panel driver would be called prior to fb driver's one.
> in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> the power off commands should be transferred to lcd panel with display
> and mipi-dsi controller enabled because the commands is set to lcd panel
> at vsync porch period. on the other hand, in opposite case, the callback
> of fb driver should be called prior to lcd panel driver's one because of
> same issue. now we could handle call order to fb blank properly.
>
> the order is as the following:
>
> at fb_blank function of fbmem.c
> -> fb_early_notifier_call_chain()
> -> lcd panel driver's early_set_power()
> -> info->fbops->fb_blank()
> -> fb driver's fb_blank()
> -> fb_notifier_call_chain()
> -> lcd panel driver's set_power()
>
I wonder if we really need the lcd_ops early_set_power callback. I can't really
imagine a situation where you need to power the LCD down only after the LCD
controller has been shutdown.
So I wonder if we couldn't just have the set_power callback, but listen to both
events and call set_power for early blank events with code != FB_BLANK_UNBLANK
and for normal blank events with code = FB_BLANK_UNBLANK?
> note that early fb blank mode is valid only if lcd_ops->early_blank_mode is 1.
> if the value is 0 then early fb blank callback would be ignored.
>
> this patch is based on git repository below:
> git://github.com/schandinat/linux-2.6.git
> branch: fbdev-next
> commit-id: a67472ad1ae040f073e45048cbc5a01195f2e3f5
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: KyungMin Park <kyungmin.park@samsung.com>
> ---
> drivers/video/backlight/lcd.c | 77 ++++++++++++++++++++++++++++++++++++++--
> drivers/video/fb_notify.c | 31 ++++++++++++++++
> drivers/video/fbmem.c | 25 +++++++------
> include/linux/fb.h | 4 ++
> include/linux/lcd.h | 61 ++++++++++++++++++++++++--------
In my opinion this should be split into two patches, one adding the early blank
event, one adding support for it to the LCD framework.
> 5 files changed, 167 insertions(+), 31 deletions(-)
>
> [...]
> diff --git a/drivers/video/fb_notify.c b/drivers/video/fb_notify.c
> index 8c02038..3930c7c 100644
> --- a/drivers/video/fb_notify.c
> +++ b/drivers/video/fb_notify.c
> @@ -13,9 +13,20 @@
> #include <linux/fb.h>
> #include <linux/notifier.h>
>
> +static BLOCKING_NOTIFIER_HEAD(fb_early_notifier_list);
> static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
>
> /**
> + * fb_register_early_client - register a client early notifier
> + * @nb: notifier block to callback on events
> + */
> +int fb_register_early_client(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&fb_early_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(fb_register_early_client);
> +
Why do we need a new notifier chain? Can't we introduce a new event for the
existing chain?
> +/**
> * fb_register_client - register a client notifier
> * @nb: notifier block to callback on events
> */
> @@ -26,6 +37,16 @@ int fb_register_client(struct notifier_block *nb)
> EXPORT_SYMBOL(fb_register_client);
>
> /**
> + * fb_unregister_early_client - unregister a client early notifier
> + * @nb: notifier block to callback on events
> + */
> +int fb_unregister_early_client(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&fb_early_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(fb_unregister_early_client);
> +
> +/**
> * fb_unregister_client - unregister a client notifier
> * @nb: notifier block to callback on events
> */
> @@ -36,6 +57,16 @@ int fb_unregister_client(struct notifier_block *nb)
> EXPORT_SYMBOL(fb_unregister_client);
>
> /**
> + * fb_early_notifier_call_chain - early notify clients of fb_events
> + *
> + */
> +int fb_early_notifier_call_chain(unsigned long val, void *v)
> +{
> + return blocking_notifier_call_chain(&fb_early_notifier_list, val, v);
> +}
> +EXPORT_SYMBOL_GPL(fb_early_notifier_call_chain);
> +
> +/**
> * fb_notifier_call_chain - notify clients of fb_events
> *
> */
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index ad93629..cf22516 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1031,24 +1031,25 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
>
> int
> fb_blank(struct fb_info *info, int blank)
> -{
> - int ret = -EINVAL;
> +{
> + struct fb_event event;
> + int ret = -EINVAL;
>
> - if (blank > FB_BLANK_POWERDOWN)
> - blank = FB_BLANK_POWERDOWN;
> + if (blank > FB_BLANK_POWERDOWN)
> + blank = FB_BLANK_POWERDOWN;
>
> - if (info->fbops->fb_blank)
> - ret = info->fbops->fb_blank(blank, info);
> + event.info = info;
> + event.data = ␣
>
> - if (!ret) {
> - struct fb_event event;
> + fb_early_notifier_call_chain(FB_EVENT_BLANK, &event);
>
> - event.info = info;
> - event.data = ␣
> + if (info->fbops->fb_blank)
> + ret = info->fbops->fb_blank(blank, info);
I think we have to handle the case where the fb_blank callback fails and should
somehow revert the effects of the early blank event.
> +
> + if (!ret)
> fb_notifier_call_chain(FB_EVENT_BLANK, &event);
> - }
>
> - return ret;
> + return ret;
> }
>
> static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 1d6836c..1d7d995 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -562,6 +562,10 @@ struct fb_blit_caps {
> u32 flags;
> };
>
> +extern int fb_register_early_client(struct notifier_block *nb);
> +extern int fb_unregister_early_client(struct notifier_block *nb);
> +extern int fb_early_notifier_call_chain(unsigned long val, void *v);
> +
> extern int fb_register_client(struct notifier_block *nb);
> extern int fb_unregister_client(struct notifier_block *nb);
> extern int fb_notifier_call_chain(unsigned long val, void *v);
> diff --git a/include/linux/lcd.h b/include/linux/lcd.h
> index 8877123..930d1cc 100644
> --- a/include/linux/lcd.h
> +++ b/include/linux/lcd.h
> @@ -37,10 +37,21 @@ struct lcd_properties {
> };
>
> struct lcd_ops {
> - /* Get the LCD panel power status (0: full on, 1..3: controller
> - power on, flat panel power off, 4: full off), see FB_BLANK_XXX */
> + /*
> + * Get the LCD panel power status (0: full on, 1..3: controller
> + * power on, flat panel power off, 4: full off), see FB_BLANK_XXX
> + */
> int (*get_power)(struct lcd_device *);
> - /* Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX) */
> + /*
> + * Get the current contrast setting (0-max_contrast) and
???
> + * Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX)
> + * this callback would be called proir to fb driver's fb_blank callback.
> + */
> + int (*early_set_power)(struct lcd_device *, int power);
> + /*
> + * Get the current contrast setting (0-max_contrast)
> + * Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX)
> + */
> int (*set_power)(struct lcd_device *, int power);
> /* Get the current contrast setting (0-max_contrast) */
> int (*get_contrast)(struct lcd_device *);
> @@ -48,21 +59,35 @@ struct lcd_ops {
> int (*set_contrast)(struct lcd_device *, int contrast);
> /* Set LCD panel mode (resolutions ...) */
> int (*set_mode)(struct lcd_device *, struct fb_videomode *);
> - /* Check if given framebuffer device is the one LCD is bound to;
> - return 0 if not, !=0 if it is. If NULL, lcd always matches the fb. */
> + /*
> + * Check if given framebuffer device is the one LCD is bound to;
> + * return 0 if not, !=0 if it is. If NULL, lcd always matches the fb.
> + */
> int (*check_fb)(struct lcd_device *, struct fb_info *);
> +
> + /*
> + * indicate whether enabling early blank mode or not.
> + * (0: disable; 1: enable);
> + * if enabled, lcd blank callback would be called prior
> + * to fb blank callback.
> + */
> + unsigned int early_blank_mode;
I think it should be sufficient to check early_set_power for NULL instead of
adding this additional flag.
> };
>
> struct lcd_device {
> struct lcd_properties props;
> - /* This protects the 'ops' field. If 'ops' is NULL, the driver that
> - registered this device has been unloaded, and if class_get_devdata()
> - points to something in the body of that driver, it is also invalid. */
> + /*
> + * This protects the 'ops' field. If 'ops' is NULL, the driver that
> + * registered this device has been unloaded, and if class_get_devdata()
> + * points to something in the body of that driver, it is also invalid.
> + */
> struct mutex ops_lock;
> /* If this is NULL, the backing module is unloaded */
> struct lcd_ops *ops;
> /* Serialise access to set_power method */
> struct mutex update_lock;
> + /* The framebuffer early notifier block */
> + struct notifier_block fb_early_notif;
> /* The framebuffer notifier block */
> struct notifier_block fb_notif;
>
> @@ -72,16 +97,22 @@ struct lcd_device {
> struct lcd_platform_data {
> /* reset lcd panel device. */
> int (*reset)(struct lcd_device *ld);
> - /* on or off to lcd panel. if 'enable' is 0 then
> - lcd power off and 1, lcd power on. */
> + /*
> + * on or off to lcd panel. if 'enable' is 0 then
> + * lcd power off and 1, lcd power on.
> + */
> int (*power_on)(struct lcd_device *ld, int enable);
>
> - /* it indicates whether lcd panel was enabled
> - from bootloader or not. */
> + /*
> + * it indicates whether lcd panel was enabled
> + * from bootloader or not.
> + */
> int lcd_enabled;
> - /* it means delay for stable time when it becomes low to high
> - or high to low that is dependent on whether reset gpio is
> - low active or high active. */
> + /*
> + * it means delay for stable time when it becomes low to high
> + * or high to low that is dependent on whether reset gpio is
> + * low active or high active.
> + */
The formatting cleanup patches should go into a separate patch.
^ permalink raw reply
* Proposal for a low-level Linux display framework
From: Tomi Valkeinen @ 2011-09-15 12:07 UTC (permalink / raw)
To: linux-fbdev, linux-kernel, dri-devel, linaro-dev
Cc: Clark, Rob, Archit Taneja
Hi,
I am the author of OMAP display driver, and while developing it I've
often felt that there's something missing in Linux's display area. I've
been planning to write a post about this for a few years already, but I
never got to it. So here goes at last!
---
First I want to (try to) describe shortly what we have on OMAP, to give
a bit of a background for my point of view, and to have an example HW.
The display subsystem (DSS) hardware on OMAP handles only showing pixels
on a display, so it doesn't contain anything that produces pixels like
3D stuff or accelerated copying. All it does is fetch pixels from SDRAM,
possibly do some modifications for them (color format conversions etc),
and output them to a display.
The hardware has multiple overlays, which are like hardware windows.
They fetch pixels from SDRAM, and output them in a certain area on the
display (possibly with scaling). Multiple overlays can be composited
into one output.
So we may have something like this, when all overlays read pixels from
separate areas in the memory, and all overlays are on LCD display:
.-----. .------. .------.
| mem |-------->| ovl0 |-----.---->| LCD |
'-----' '------' | '------'
.-----. .------. |
| mem |-------->| ovl1 |-----|
'-----' '------' |
.-----. .------. | .------.
| mem |-------->| ovl2 |-----' | TV |
'-----' '------' '------'
The LCD display can be rather simple one, like a standard monitor or a
simple panel directly connected to parallel RGB output, or a more
complex one. A complex panel needs something else than just
turn-it-on-and-go. This may involve sending and receiving messages
between OMAP and the panel, but more generally, there's need to have
custom code that handles the particular panel. And the complex panel is
not necessarily a panel at all, it may be a buffer chip between OMAP and
the actual panel.
The software side can be divided into three parts: the lower level
omapdss driver, the lower level panel drivers, and higher level drivers
like omapfb, v4l2 and omapdrm.
The omapdss driver handles the OMAP DSS hardware, and offers a kernel
internal API which the higher level drivers use. The omapdss does not
know anything about fb or drm, it just offers core display services.
The panel drivers handle particular panels/chips. The panel driver may
be very simple in case of a conventional display, basically doing pretty
much nothing, or bigger piece of code, handling communication with the
panel.
The higher level drivers handle buffers and tell omapdss things like
where to find the pixels, what size the overlays should be, and use the
omapdss API to turn displays on/off, etc.
---
There are two things that I'm proposing to improve the Linux display
support:
First, there should be a bunch of common video structs and helpers that
are independent of any higher level framework. Things like video
timings, mode databases, and EDID seem to be implemented multiple times
in the kernel. But there shouldn't be anything in those things that
depend on any particular display framework, so they could be implemented
just once and all the frameworks could use them.
Second, I think there could be use for a common low level display
framework. Currently the lower level code (display HW handling, etc.)
and higher level code (buffer management, policies, etc) seem to be
usually tied together, like the fb framework or the drm. Granted, the
frameworks do not force that, and for OMAP we indeed have omapfb and
omapdrm using the lower level omapdss. But I don't see that it's
anything OMAP specific as such.
I think the lower level framework could have components something like
this (the naming is OMAP oriented, of course):
overlay - a hardware "window", gets pixels from memory, possibly does
format conversions, scaling, etc.
overlay compositor - composes multiple overlays into one output,
possibly doing things like translucency.
output - gets the pixels from overlay compositor, and sends them out
according to particular video timings when using conventional video
interface, or via any other mean when using non-conventional video buses
like DSI command mode.
display - handles an external display. For conventional displays this
wouldn't do much, but for complex ones it does whatever needed by that
particular display.
This is something similar to what DRM has, I believe. The biggest
difference is that the display can be a full blown driver for a complex
piece of HW.
This kind of low level framework would be good for two purposes: 1) I
think it's a good division generally, having the low level HW driver
separate from the higher level buffer/policy management and 2) fb, drm,
v4l2 or any possible future framework could all use the same low level
framework.
---
Now, I'm quite sure the above framework could work quite well with any
OMAP like hardware, with unified memory (i.e. the video buffers are in
SDRAM) and 3D chips and similar components are separate. But what I'm
not sure is how desktop world's gfx cards change things. Most probably
all the above components can be found from there also in some form, but
are there some interdependencies between 3D/buffer management/something
else and the video output side?
This was a very rough and quite short proposal, but I'm happy to improve
and extend it if it's not totally shot down.
Tomi
^ permalink raw reply
* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-15 14:59 UTC (permalink / raw)
To: Tomi Valkeinen, linux-fbdev, linux-kernel, dri-devel, linaro-dev
Cc: Clark, Rob, Archit Taneja
In-Reply-To: <1316088425.11294.78.camel@lappyti>
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
On Thu, 15 Sep 2011 15:07:05 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> This was a very rough and quite short proposal, but I'm happy to improve
> and extend it if it's not totally shot down.
Jesse Barnes has put together a proposal much like this to work within
the existing DRM environment. This is pretty much the last piece of
missing mode-setting functionality that we know of, making DRM capable
of fully supporting existing (and planned) devices.
Here's a link to some older discussion on the issue, things have changed
a bit since then and we had a long talk about this during the X
Developers' Conference this week in Chicago. Expect an update to his
proposal in the coming weeks.
http://lists.freedesktop.org/archives/dri-devel/2011-April/010559.html
--
keith.packard@intel.com
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Proposal for a low-level Linux display framework
From: Kyungmin Park @ 2011-09-15 15:03 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-fbdev, linux-kernel, dri-devel, linaro-dev, Archit Taneja,
대인기
In-Reply-To: <1316088425.11294.78.camel@lappyti>
Hi Tomi,
On Thu, Sep 15, 2011 at 9:07 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> I am the author of OMAP display driver, and while developing it I've
> often felt that there's something missing in Linux's display area. I've
> been planning to write a post about this for a few years already, but I
> never got to it. So here goes at last!
>
> ---
>
> First I want to (try to) describe shortly what we have on OMAP, to give
> a bit of a background for my point of view, and to have an example HW.
>
> The display subsystem (DSS) hardware on OMAP handles only showing pixels
> on a display, so it doesn't contain anything that produces pixels like
> 3D stuff or accelerated copying. All it does is fetch pixels from SDRAM,
> possibly do some modifications for them (color format conversions etc),
> and output them to a display.
>
> The hardware has multiple overlays, which are like hardware windows.
> They fetch pixels from SDRAM, and output them in a certain area on the
> display (possibly with scaling). Multiple overlays can be composited
> into one output.
>
> So we may have something like this, when all overlays read pixels from
> separate areas in the memory, and all overlays are on LCD display:
>
> .-----. .------. .------.
> | mem |-------->| ovl0 |-----.---->| LCD |
> '-----' '------' | '------'
> .-----. .------. |
> | mem |-------->| ovl1 |-----|
> '-----' '------' |
> .-----. .------. | .------.
> | mem |-------->| ovl2 |-----' | TV |
> '-----' '------' '------'
>
Same feature at samsung display subsystem.
> The LCD display can be rather simple one, like a standard monitor or a
> simple panel directly connected to parallel RGB output, or a more
> complex one. A complex panel needs something else than just
> turn-it-on-and-go. This may involve sending and receiving messages
> between OMAP and the panel, but more generally, there's need to have
> custom code that handles the particular panel. And the complex panel is
> not necessarily a panel at all, it may be a buffer chip between OMAP and
> the actual panel.
>
> The software side can be divided into three parts: the lower level
> omapdss driver, the lower level panel drivers, and higher level drivers
> like omapfb, v4l2 and omapdrm.
Current omapdrm codes use the omapfb and omapdss codes even though
omapdrm is located drivers/staging, some time later it should be
drivers/gpu/gem/omap. but it still uses the drivers/video/omap2/dss
codes.
In case of samsung DRM, it has almost similar codes for lowlevel
access from the drivers/video/s3c-fb.c for FIMD and
drivers/media/video/s5p-tv for HDMI.
>
> The omapdss driver handles the OMAP DSS hardware, and offers a kernel
> internal API which the higher level drivers use. The omapdss does not
> know anything about fb or drm, it just offers core display services.
>
> The panel drivers handle particular panels/chips. The panel driver may
> be very simple in case of a conventional display, basically doing pretty
> much nothing, or bigger piece of code, handling communication with the
> panel.
>
> The higher level drivers handle buffers and tell omapdss things like
> where to find the pixels, what size the overlays should be, and use the
> omapdss API to turn displays on/off, etc.
>
> ---
>
> There are two things that I'm proposing to improve the Linux display
> support:
>
> First, there should be a bunch of common video structs and helpers that
> are independent of any higher level framework. Things like video
> timings, mode databases, and EDID seem to be implemented multiple times
> in the kernel. But there shouldn't be anything in those things that
> depend on any particular display framework, so they could be implemented
> just once and all the frameworks could use them.
>
> Second, I think there could be use for a common low level display
> framework. Currently the lower level code (display HW handling, etc.)
> and higher level code (buffer management, policies, etc) seem to be
> usually tied together, like the fb framework or the drm. Granted, the
> frameworks do not force that, and for OMAP we indeed have omapfb and
> omapdrm using the lower level omapdss. But I don't see that it's
> anything OMAP specific as such.
So I suggest the create the drivers/graphics for lowlevel codes and
each framework, DRM, V4L2 and FB uses these lowlevel codes.
Thank you,
Kyungmin Park
>
> I think the lower level framework could have components something like
> this (the naming is OMAP oriented, of course):
>
> overlay - a hardware "window", gets pixels from memory, possibly does
> format conversions, scaling, etc.
>
> overlay compositor - composes multiple overlays into one output,
> possibly doing things like translucency.
>
> output - gets the pixels from overlay compositor, and sends them out
> according to particular video timings when using conventional video
> interface, or via any other mean when using non-conventional video buses
> like DSI command mode.
>
> display - handles an external display. For conventional displays this
> wouldn't do much, but for complex ones it does whatever needed by that
> particular display.
>
> This is something similar to what DRM has, I believe. The biggest
> difference is that the display can be a full blown driver for a complex
> piece of HW.
>
> This kind of low level framework would be good for two purposes: 1) I
> think it's a good division generally, having the low level HW driver
> separate from the higher level buffer/policy management and 2) fb, drm,
> v4l2 or any possible future framework could all use the same low level
> framework.
>
> ---
>
> Now, I'm quite sure the above framework could work quite well with any
> OMAP like hardware, with unified memory (i.e. the video buffers are in
> SDRAM) and 3D chips and similar components are separate. But what I'm
> not sure is how desktop world's gfx cards change things. Most probably
> all the above components can be found from there also in some form, but
> are there some interdependencies between 3D/buffer management/something
> else and the video output side?
>
> This was a very rough and quite short proposal, but I'm happy to improve
> and extend it if it's not totally shot down.
>
> Tomi
>
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
^ permalink raw reply
* Re: Proposal for a low-level Linux display framework
From: Tomi Valkeinen @ 2011-09-15 15:29 UTC (permalink / raw)
To: Keith Packard
Cc: linux-fbdev, linaro-dev, linux-kernel, dri-devel, Archit Taneja,
Clark, Rob
In-Reply-To: <yunboul7sx7.fsf@aiko.keithp.com>
On Thu, 2011-09-15 at 09:59 -0500, Keith Packard wrote:
> On Thu, 15 Sep 2011 15:07:05 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> > This was a very rough and quite short proposal, but I'm happy to improve
> > and extend it if it's not totally shot down.
>
> Jesse Barnes has put together a proposal much like this to work within
> the existing DRM environment. This is pretty much the last piece of
> missing mode-setting functionality that we know of, making DRM capable
> of fully supporting existing (and planned) devices.
>
> Here's a link to some older discussion on the issue, things have changed
> a bit since then and we had a long talk about this during the X
> Developers' Conference this week in Chicago. Expect an update to his
> proposal in the coming weeks.
>
> http://lists.freedesktop.org/archives/dri-devel/2011-April/010559.html
>
Thanks for the link.
Right, DRM has already components I described in my proposal, and adding
overlays brings it even closer. However, I think there are two major
differences:
1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
the plan is to make DRM the core Linux display framework, upon which
everything else is built, and fb and v4l2 are changed to use DRM.
But even if it was done like that, I see that it's combining two
separate things: 1) the lower level HW control, and 2) the upper level
buffer management, policies and userspace interfaces.
2) It's missing the panel driver part. This is rather important on
embedded systems, as the panels often are not "dummy" panels, but they
need things like custom initialization, sending commands to adjust
backlight, etc.
Tomi
^ permalink raw reply
* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-15 15:50 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-fbdev, linux-kernel, dri-devel, linaro-dev, Clark, Rob,
Archit Taneja
In-Reply-To: <1316100594.23214.65.camel@deskari>
[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]
On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
> the plan is to make DRM the core Linux display framework, upon which
> everything else is built, and fb and v4l2 are changed to use DRM.
I'd like to think we could make DRM the underlying display framework;
it already exposes an fb interface, and with overlays, a bit more of the
v4l2 stuff is done as well. Certainly eliminating three copies of mode
setting infrastructure would be nice...
> But even if it was done like that, I see that it's combining two
> separate things: 1) the lower level HW control, and 2) the upper level
> buffer management, policies and userspace interfaces.
Those are split between the DRM layer and the underlying device driver,
which provides both kernel (via fb) and user space interfaces.
> 2) It's missing the panel driver part. This is rather important on
> embedded systems, as the panels often are not "dummy" panels, but they
> need things like custom initialization, sending commands to adjust
> backlight, etc.
We integrate the panel (and other video output) drivers into the device
drivers. With desktop chips, they're not easily separable. None of the
desktop output drivers are simple; things like DisplayPort require link
training, and everyone needs EDID. We share some of that code in the DRM
layer today, and it would be nice to share even more.
We should figure out if the DRM interfaces are sufficient for your
needs; they're pretty flexible at this point.
Of course, backlight remains a mess in the desktop world; with many
custom backlight drivers along with generic ACPI and then
per-video-device drivers as well.
--
keith.packard@intel.com
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ 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