* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-12 13:36 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <1315818818-18733-10-git-send-email-tomi.valkeinen@ti.com>
Hi Tomi,
On Mon, Sep 12, 2011 at 2:43 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Implement detect() by checking the hot plug detect status.
>
> The implementation is not very good, as it always turns on the HDMI
> output to get the detection working. HDMI driver needs improvements so
> that we could enable only core parts of it.
>
> Cc: Mythri P K <mythripk@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dss.h | 1 +
> drivers/video/omap2/dss/dss_features.c | 1 +
> drivers/video/omap2/dss/hdmi.c | 17 +++++++++++++++++
> drivers/video/omap2/dss/hdmi_panel.c | 25 +++++++++++++++++++++++++
> drivers/video/omap2/dss/ti_hdmi.h | 3 +++
> drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 12 ++++++++++++
> 6 files changed, 59 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 2e7799c..f58c302 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -495,6 +495,7 @@ void omapdss_hdmi_display_set_timing(struct omap_dss_device *dssdev);
> int omapdss_hdmi_display_check_timing(struct omap_dss_device *dssdev,
> struct omap_video_timings *timings);
> int omapdss_hdmi_read_edid(u8 *buf, int len);
> +bool omapdss_hdmi_detect(void);
> int hdmi_panel_init(void);
> void hdmi_panel_exit(void);
>
> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
> index 076f399..ab41665 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -440,6 +440,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = {
> .phy_enable = ti_hdmi_4xxx_phy_enable,
> .phy_disable = ti_hdmi_4xxx_phy_disable,
> .read_edid = ti_hdmi_4xxx_read_edid,
> + .detect = ti_hdmi_4xxx_detect,
> .pll_enable = ti_hdmi_4xxx_pll_enable,
> .pll_disable = ti_hdmi_4xxx_pll_disable,
> .video_enable = ti_hdmi_4xxx_wp_video_start,
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index fb85ce5..7818670 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -449,6 +449,23 @@ int omapdss_hdmi_read_edid(u8 *buf, int len)
> return r;
> }
>
> +bool omapdss_hdmi_detect(void)
> +{
> + int r;
> +
> + mutex_lock(&hdmi.lock);
> +
> + r = hdmi_runtime_get();
> + BUG_ON(r);
> +
> + r = hdmi.ip_data.ops->detect(&hdmi.ip_data);
> +
> + hdmi_runtime_put();
> + mutex_unlock(&hdmi.lock);
> +
> + return r = 1;
> +}
> +
> int omapdss_hdmi_display_enable(struct omap_dss_device *dssdev)
> {
> int r = 0;
> diff --git a/drivers/video/omap2/dss/hdmi_panel.c b/drivers/video/omap2/dss/hdmi_panel.c
> index 71aa813..533d5dc 100644
> --- a/drivers/video/omap2/dss/hdmi_panel.c
> +++ b/drivers/video/omap2/dss/hdmi_panel.c
> @@ -25,6 +25,7 @@
> #include <linux/mutex.h>
> #include <linux/module.h>
> #include <video/omapdss.h>
> +#include <linux/slab.h>
>
> #include "dss.h"
>
> @@ -198,6 +199,29 @@ err:
> return r;
> }
>
> +static bool hdmi_detect(struct omap_dss_device *dssdev)
> +{
> + int r;
> +
> + mutex_lock(&hdmi.hdmi_lock);
> +
> + if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE) {
> + r = omapdss_hdmi_display_enable(dssdev);
> + if (r)
> + goto err;
> + }
> +
> + r = omapdss_hdmi_detect();
> +
> + if (dssdev->state = OMAP_DSS_DISPLAY_DISABLED ||
> + dssdev->state = OMAP_DSS_DISPLAY_SUSPENDED)
> + omapdss_hdmi_display_disable(dssdev);
> +err:
> + mutex_unlock(&hdmi.hdmi_lock);
> +
> + return r;
> +}
> +
> static struct omap_dss_driver hdmi_driver = {
> .probe = hdmi_panel_probe,
> .remove = hdmi_panel_remove,
> @@ -209,6 +233,7 @@ static struct omap_dss_driver hdmi_driver = {
> .set_timings = hdmi_set_timings,
> .check_timings = hdmi_check_timings,
> .read_edid = hdmi_read_edid,
> + .detect = hdmi_detect,
> .driver = {
> .name = "hdmi_panel",
> .owner = THIS_MODULE,
> diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
> index 390cd85b..d48603c 100644
> --- a/drivers/video/omap2/dss/ti_hdmi.h
> +++ b/drivers/video/omap2/dss/ti_hdmi.h
> @@ -94,6 +94,8 @@ struct ti_hdmi_ip_ops {
>
> int (*read_edid)(struct hdmi_ip_data *ip_data, u8 *edid, int len);
>
> + bool (*detect)(struct hdmi_ip_data *ip_data);
> +
> int (*pll_enable)(struct hdmi_ip_data *ip_data);
>
> void (*pll_disable)(struct hdmi_ip_data *ip_data);
> @@ -114,6 +116,7 @@ struct hdmi_ip_data {
> int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
> void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
> int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data, u8 *edid, int len);
> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data);
> void ti_hdmi_4xxx_wp_video_start(struct hdmi_ip_data *ip_data, bool start);
> int ti_hdmi_4xxx_pll_enable(struct hdmi_ip_data *ip_data);
> void ti_hdmi_4xxx_pll_disable(struct hdmi_ip_data *ip_data);
> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> index e9885dc..da7fe50 100644
> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> @@ -416,6 +416,18 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
> return l;
> }
>
> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> +{
> + int r;
> +
> + void __iomem *base = hdmi_core_sys_base(ip_data);
> +
> + /* HPD */
> + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> +
> + return r = 1;
> +}
> +
For HPD the probe should also be on the core interrupt first , and the
detect should be dynamic, ie based on the cable connect and disconnect
event.So this approach for HPD is not really the way.
Also that should be based on the GPIO(63) , I am planning to push a
patch on that shortly.
> static void hdmi_core_init(struct hdmi_core_video_config *video_cfg,
> struct hdmi_core_infoframe_avi *avi_cfg,
> struct hdmi_core_packet_enable_repeat *repeat_cfg)
> --
> 1.7.4.1
>
>
Thanks and regards,
Mythri.
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-12 16:18 UTC (permalink / raw)
To: K, Mythri P; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9_Jsg23oR49q2bkBGVLDWHBiBZj_=_G1SRf7ftrMt_iw@mail.gmail.com>
On Mon, 2011-09-12 at 18:54 +0530, K, Mythri P wrote:
> Hi Tomi,
>
> On Mon, Sep 12, 2011 at 2:43 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > Implement detect() by checking the hot plug detect status.
> >
> > The implementation is not very good, as it always turns on the HDMI
> > output to get the detection working. HDMI driver needs improvements so
> > that we could enable only core parts of it.
> >
> > Cc: Mythri P K <mythripk@ti.com>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > ---
<snip>
> > +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> > +{
> > + int r;
> > +
> > + void __iomem *base = hdmi_core_sys_base(ip_data);
> > +
> > + /* HPD */
> > + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> > +
> > + return r = 1;
> > +}
> > +
> For HPD the probe should also be on the core interrupt first , and the
> detect should be dynamic, ie based on the cable connect and disconnect
> event.So this approach for HPD is not really the way.
This is not for the event, this is for polling. There is currently no
hot plug event mechanism in the DSS.
Do we get an interrupt when the driver is loaded and the cable is
already connected? And do you plan to keep the plugged in/out state
stored somewhere, or how do you implement detect()?
> Also that should be based on the GPIO(63) , I am planning to push a
> patch on that shortly.
What is gpio 63? What does the HDMI_CORE_SYS_STAT HDP bit tell us then?
Tomi
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Rob Clark @ 2011-09-12 16:46 UTC (permalink / raw)
To: K, Mythri P; +Cc: Tomi Valkeinen, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9_Jsg23oR49q2bkBGVLDWHBiBZj_=_G1SRf7ftrMt_iw@mail.gmail.com>
On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> +{
>> + int r;
>> +
>> + void __iomem *base = hdmi_core_sys_base(ip_data);
>> +
>> + /* HPD */
>> + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>> +
>> + return r = 1;
>> +}
>> +
> For HPD the probe should also be on the core interrupt first , and the
> detect should be dynamic, ie based on the cable connect and disconnect
> event.So this approach for HPD is not really the way.
> Also that should be based on the GPIO(63) , I am planning to push a
> patch on that shortly.
Fwiw, we do still need a dssdrv->detect() function from omapdrm
driver.. if there is another way to implement that function, such as
with a GPIO, that is great. But somehow or another we need the detect
function. The implementation can always change later.
BR,
-R
^ permalink raw reply
* [REPOST][PATCH 1/2] fbdev: fix indentation in modedb.c
From: Timur Tabi @ 2011-09-13 16:05 UTC (permalink / raw)
To: linux-fbdev
Fix the incorrect indentation in functions fb_try_mode() and fb_find_mode().
Signed-off-by: Timur Tabi <timur@freescale.com>
---
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;
}
/**
--
1.7.3.4
^ permalink raw reply related
* [REPOST][PATCH 2/2] video: miscellaneous minor changes to the Freescale DIU driver
From: Timur Tabi @ 2011-09-13 16:05 UTC (permalink / raw)
To: linux-fbdev
Make several minor, miscellaneous changes to the Freescale DIU framebuffer
driver. These changes "lighten" the code by removing crud, fixing small
bugs, and fixing some coding style problems. These changes will make it
easier to make more substantial fixes in the future.
1. Fix incorrect indentation and spacing with some code.
2. Remove debug printks (they don't actually help in debugging the code).
3. Clean up some other printks (e.g. use pr_xxx, clean up the text, etc).
4. Remove the "default" videomode object since it's just a dupe of the
first element in the videomode array.
5. Remove some superfluous local variables.
6. Rename ofdev to pdev, since it's a platform device not an OF device.
7. Fix some device tree operations.
8. Fix some build warnings.
9. Removed some unused structures from the header file.
10. Other minor bug fixes and changes.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/video/fsl-diu-fb.c | 362 +++++++++++++++-----------------------------
include/linux/fsl-diu-fb.h | 12 +--
2 files changed, 125 insertions(+), 249 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 0f1933b..1470581 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -36,26 +36,10 @@
#include "edid.h"
/*
- * These parameters give default parameters
- * for video output 1024x768,
- * FIXME - change timing to proper amounts
- * hsync 31.5kHz, vsync 60Hz
+ * List of supported video modes
+ *
+ * The first entry is the default video mode
*/
-static struct fb_videomode __devinitdata fsl_diu_default_mode = {
- .refresh = 60,
- .xres = 1024,
- .yres = 768,
- .pixclock = 15385,
- .left_margin = 160,
- .right_margin = 24,
- .upper_margin = 29,
- .lower_margin = 3,
- .hsync_len = 136,
- .vsync_len = 6,
- .sync = FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
- .vmode = FB_VMODE_NONINTERLACED
-};
-
static struct fb_videomode __devinitdata fsl_diu_mode_db[] = {
{
.name = "1024x768-60",
@@ -217,59 +201,59 @@ struct mfb_info {
int x_aoi_d; /* aoi display x offset to physical screen */
int y_aoi_d; /* aoi display y offset to physical screen */
struct fsl_diu_data *parent;
- u8 *edid_data;
+ void *edid_data;
};
static struct mfb_info mfb_template[] = {
{ /* AOI 0 for plane 0 */
- .index = 0,
- .type = MFB_TYPE_OUTPUT,
- .id = "Panel0",
- .registered = 0,
- .count = 0,
- .x_aoi_d = 0,
- .y_aoi_d = 0,
+ .index = 0,
+ .type = MFB_TYPE_OUTPUT,
+ .id = "Panel0",
+ .registered = 0,
+ .count = 0,
+ .x_aoi_d = 0,
+ .y_aoi_d = 0,
},
{ /* AOI 0 for plane 1 */
- .index = 1,
- .type = MFB_TYPE_OUTPUT,
- .id = "Panel1 AOI0",
- .registered = 0,
- .g_alpha = 0xff,
- .count = 0,
- .x_aoi_d = 0,
- .y_aoi_d = 0,
+ .index = 1,
+ .type = MFB_TYPE_OUTPUT,
+ .id = "Panel1 AOI0",
+ .registered = 0,
+ .g_alpha = 0xff,
+ .count = 0,
+ .x_aoi_d = 0,
+ .y_aoi_d = 0,
},
{ /* AOI 1 for plane 1 */
- .index = 2,
- .type = MFB_TYPE_OUTPUT,
- .id = "Panel1 AOI1",
- .registered = 0,
- .g_alpha = 0xff,
- .count = 0,
- .x_aoi_d = 0,
- .y_aoi_d = 480,
+ .index = 2,
+ .type = MFB_TYPE_OUTPUT,
+ .id = "Panel1 AOI1",
+ .registered = 0,
+ .g_alpha = 0xff,
+ .count = 0,
+ .x_aoi_d = 0,
+ .y_aoi_d = 480,
},
{ /* AOI 0 for plane 2 */
- .index = 3,
- .type = MFB_TYPE_OUTPUT,
- .id = "Panel2 AOI0",
- .registered = 0,
- .g_alpha = 0xff,
- .count = 0,
- .x_aoi_d = 640,
- .y_aoi_d = 0,
+ .index = 3,
+ .type = MFB_TYPE_OUTPUT,
+ .id = "Panel2 AOI0",
+ .registered = 0,
+ .g_alpha = 0xff,
+ .count = 0,
+ .x_aoi_d = 640,
+ .y_aoi_d = 0,
},
{ /* AOI 1 for plane 2 */
- .index = 4,
- .type = MFB_TYPE_OUTPUT,
- .id = "Panel2 AOI1",
- .registered = 0,
- .g_alpha = 0xff,
- .count = 0,
- .x_aoi_d = 640,
- .y_aoi_d = 480,
+ .index = 4,
+ .type = MFB_TYPE_OUTPUT,
+ .id = "Panel2 AOI1",
+ .registered = 0,
+ .g_alpha = 0xff,
+ .count = 0,
+ .x_aoi_d = 640,
+ .y_aoi_d = 480,
},
};
@@ -322,14 +306,9 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
{
void *virt;
- pr_debug("size=%zu\n", size);
-
virt = alloc_pages_exact(size, GFP_DMA | __GFP_ZERO);
- if (virt) {
+ if (virt)
*phys = virt_to_phys(virt);
- pr_debug("virt=%p phys=%llx\n", virt,
- (unsigned long long)*phys);
- }
return virt;
}
@@ -343,8 +322,6 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
*/
static void fsl_diu_free(void *virt, size_t size)
{
- pr_debug("virt=%p size=%zu\n", virt, size);
-
if (virt && size)
free_pages_exact(virt, size);
}
@@ -368,7 +345,6 @@ static int fsl_diu_enable_panel(struct fb_info *info)
struct fsl_diu_data *machine_data = mfbi->parent;
int res = 0;
- pr_debug("enable_panel index %d\n", mfbi->index);
if (mfbi->type != MFB_TYPE_OFF) {
switch (mfbi->index) {
case 0: /* plane 0 */
@@ -585,9 +561,6 @@ static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
static int fsl_diu_check_var(struct fb_var_screeninfo *var,
struct fb_info *info)
{
- pr_debug("check_var xres: %d\n", var->xres);
- pr_debug("check_var yres: %d\n", var->yres);
-
if (var->xres_virtual < var->xres)
var->xres_virtual = var->xres;
if (var->yres_virtual < var->yres)
@@ -682,7 +655,7 @@ static void set_fix(struct fb_info *info)
struct fb_var_screeninfo *var = &info->var;
struct mfb_info *mfbi = info->par;
- strncpy(fix->id, mfbi->id, strlen(mfbi->id));
+ strncpy(fix->id, mfbi->id, sizeof(fix->id));
fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
fix->type = FB_TYPE_PACKED_PIXELS;
fix->accel = FB_ACCEL_NONE;
@@ -715,12 +688,11 @@ static void update_lcdc(struct fb_info *info)
/* Prep for DIU init - gamma table, cursor table */
for (i = 0; i <= 2; i++)
- for (j = 0; j <= 255; j++)
- *gamma_table_base++ = j;
+ for (j = 0; j <= 255; j++)
+ *gamma_table_base++ = j;
diu_ops.set_gamma_table(machine_data->monitor_port, pool.gamma.vaddr);
- pr_debug("update-lcdc: HW - %p\n Disabling DIU\n", hw);
disable_lcdc(info);
/* Program DIU registers */
@@ -732,9 +704,6 @@ static void update_lcdc(struct fb_info *info)
out_be32(&hw->bgnd_wb, 0); /* BGND_WB */
out_be32(&hw->disp_size, (var->yres << 16 | var->xres));
/* DISP SIZE */
- pr_debug("DIU xres: %d\n", var->xres);
- pr_debug("DIU yres: %d\n", var->yres);
-
out_be32(&hw->wb_size, 0); /* WB SIZE */
out_be32(&hw->wb_mem_addr, 0); /* WB MEM ADDR */
@@ -751,15 +720,6 @@ static void update_lcdc(struct fb_info *info)
out_be32(&hw->vsyn_para, temp);
- pr_debug("DIU right_margin - %d\n", var->right_margin);
- pr_debug("DIU left_margin - %d\n", var->left_margin);
- pr_debug("DIU hsync_len - %d\n", var->hsync_len);
- pr_debug("DIU upper_margin - %d\n", var->upper_margin);
- pr_debug("DIU lower_margin - %d\n", var->lower_margin);
- pr_debug("DIU vsync_len - %d\n", var->vsync_len);
- pr_debug("DIU HSYNC - 0x%08x\n", hw->hsyn_para);
- pr_debug("DIU VSYNC - 0x%08x\n", hw->vsyn_para);
-
diu_ops.set_pixel_clock(var->pixclock);
out_be32(&hw->syn_pol, 0); /* SYNC SIGNALS POLARITY */
@@ -776,14 +736,9 @@ static int map_video_memory(struct fb_info *info)
phys_addr_t phys;
u32 smem_len = info->fix.line_length * info->var.yres_virtual;
- pr_debug("info->var.xres_virtual = %d\n", info->var.xres_virtual);
- pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
- pr_debug("info->fix.line_length = %d\n", info->fix.line_length);
- pr_debug("MAP_VIDEO_MEMORY: smem_len = %u\n", smem_len);
-
info->screen_base = fsl_diu_alloc(smem_len, &phys);
if (info->screen_base = NULL) {
- printk(KERN_ERR "Unable to allocate fb memory\n");
+ dev_err(info->dev, "unable to allocate fb memory\n");
return -ENOMEM;
}
mutex_lock(&info->mm_lock);
@@ -792,10 +747,6 @@ static int map_video_memory(struct fb_info *info)
mutex_unlock(&info->mm_lock);
info->screen_size = info->fix.smem_len;
- pr_debug("Allocated fb @ paddr=0x%08lx, size=%d.\n",
- info->fix.smem_start, info->fix.smem_len);
- pr_debug("screen base %p\n", info->screen_base);
-
return 0;
}
@@ -852,11 +803,10 @@ static int fsl_diu_set_par(struct fb_info *info)
if (len != info->fix.smem_len) {
if (info->fix.smem_start)
unmap_video_memory(info);
- pr_debug("SET PAR: smem_len = %d\n", info->fix.smem_len);
/* Memory allocation for framebuffer */
if (map_video_memory(info)) {
- printk(KERN_ERR "Unable to allocate fb memory 1\n");
+ dev_err(info->dev, "unable to allocate fb memory 1\n");
return -ENOMEM;
}
}
@@ -887,7 +837,7 @@ static int fsl_diu_set_par(struct fb_info *info)
static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
{
- return ((val<<width) + 0x7FFF - val)>>16;
+ return ((val << width) + 0x7FFF - val) >> 16;
}
/*
@@ -899,8 +849,9 @@ static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
* pseudo_palette in struct fb_info. For pseudocolor mode we have a limited
* color palette.
*/
-static int fsl_diu_setcolreg(unsigned regno, unsigned red, unsigned green,
- unsigned blue, unsigned transp, struct fb_info *info)
+static int fsl_diu_setcolreg(unsigned int regno, unsigned int red,
+ unsigned int green, unsigned int blue,
+ unsigned int transp, struct fb_info *info)
{
int ret = 1;
@@ -935,9 +886,6 @@ static int fsl_diu_setcolreg(unsigned regno, unsigned red, unsigned green,
ret = 0;
}
break;
- case FB_VISUAL_STATIC_PSEUDOCOLOR:
- case FB_VISUAL_PSEUDOCOLOR:
- break;
}
return ret;
@@ -1022,21 +970,17 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
if (copy_from_user(&pix_fmt, buf, sizeof(pix_fmt)))
return -EFAULT;
ad->pix_fmt = pix_fmt;
- pr_debug("Set pixel format to 0x%08x\n", ad->pix_fmt);
break;
case MFB_GET_PIXFMT:
pix_fmt = ad->pix_fmt;
if (copy_to_user(buf, &pix_fmt, sizeof(pix_fmt)))
return -EFAULT;
- pr_debug("get pixel format 0x%08x\n", ad->pix_fmt);
break;
case MFB_SET_AOID:
if (copy_from_user(&aoi_d, buf, sizeof(aoi_d)))
return -EFAULT;
mfbi->x_aoi_d = aoi_d.x_aoi_d;
mfbi->y_aoi_d = aoi_d.y_aoi_d;
- pr_debug("set AOI display offset of index %d to (%d,%d)\n",
- mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
fsl_diu_check_var(&info->var, info);
fsl_diu_set_aoi(info);
break;
@@ -1045,14 +989,11 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
aoi_d.y_aoi_d = mfbi->y_aoi_d;
if (copy_to_user(buf, &aoi_d, sizeof(aoi_d)))
return -EFAULT;
- pr_debug("get AOI display offset of index %d (%d,%d)\n",
- mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
break;
case MFB_GET_ALPHA:
global_alpha = mfbi->g_alpha;
if (copy_to_user(buf, &global_alpha, sizeof(global_alpha)))
return -EFAULT;
- pr_debug("get global alpha of index %d\n", mfbi->index);
break;
case MFB_SET_ALPHA:
/* set panel information */
@@ -1061,7 +1002,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
ad->src_size_g_alpha = (ad->src_size_g_alpha & (~0xff)) |
(global_alpha & 0xff);
mfbi->g_alpha = global_alpha;
- pr_debug("set global alpha for index %d\n", mfbi->index);
break;
case MFB_SET_CHROMA_KEY:
/* set panel winformation */
@@ -1089,7 +1029,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
ad->ckmin_g = ck.green_min;
ad->ckmin_b = ck.blue_min;
}
- pr_debug("set chroma key\n");
break;
case FBIOGET_GWINFO:
if (mfbi->type = MFB_TYPE_OFF)
@@ -1098,18 +1037,9 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
if (copy_to_user(buf, ad, sizeof(*ad)))
return -EFAULT;
break;
- case FBIOGET_HWCINFO:
- pr_debug("FBIOGET_HWCINFO:0x%08x\n", FBIOGET_HWCINFO);
- break;
- case FBIOPUT_MODEINFO:
- pr_debug("FBIOPUT_MODEINFO:0x%08x\n", FBIOPUT_MODEINFO);
- break;
- case FBIOGET_DISPINFO:
- pr_debug("FBIOGET_DISPINFO:0x%08x\n", FBIOGET_DISPINFO);
- break;
default:
- printk(KERN_ERR "Unknown ioctl command (0x%08X)\n", cmd);
+ dev_err(info->dev, "unknown ioctl command (0x%08X)\n", cmd);
return -ENOIOCTLCMD;
}
@@ -1130,7 +1060,6 @@ static int fsl_diu_open(struct fb_info *info, int user)
spin_lock(&diu_lock);
mfbi->count++;
if (mfbi->count = 1) {
- pr_debug("open plane index %d\n", mfbi->index);
fsl_diu_check_var(&info->var, info);
res = fsl_diu_set_par(info);
if (res < 0)
@@ -1156,7 +1085,6 @@ static int fsl_diu_release(struct fb_info *info, int user)
spin_lock(&diu_lock);
mfbi->count--;
if (mfbi->count = 0) {
- pr_debug("release plane index %d\n", mfbi->index);
res = fsl_diu_disable_panel(info);
if (res < 0)
mfbi->count++;
@@ -1221,26 +1149,9 @@ static int __devinit install_fb(struct fb_info *info)
} else {
aoi_mode = init_aoi_mode;
}
- pr_debug("mode used = %s\n", aoi_mode);
- rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize,
- &fsl_diu_default_mode, default_bpp);
- switch (rc) {
- case 1:
- pr_debug("using mode specified in @mode\n");
- break;
- case 2:
- pr_debug("using mode specified in @mode "
- "with ignored refresh rate\n");
- break;
- case 3:
- pr_debug("using mode default mode\n");
- break;
- case 4:
- pr_debug("using mode from list\n");
- break;
- default:
- pr_debug("rc = %d\n", rc);
- pr_debug("failed to find mode\n");
+ rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize, NULL,
+ default_bpp);
+ if (!rc) {
/*
* For plane 0 we continue and look into
* driver's internal modedb.
@@ -1249,15 +1160,12 @@ static int __devinit install_fb(struct fb_info *info)
has_default_mode = 0;
else
return -EINVAL;
- break;
}
if (!has_default_mode) {
rc = fb_find_mode(&info->var, info, aoi_mode, fsl_diu_mode_db,
- ARRAY_SIZE(fsl_diu_mode_db),
- &fsl_diu_default_mode,
- default_bpp);
- if (rc > 0 && rc < 5)
+ ARRAY_SIZE(fsl_diu_mode_db), NULL, default_bpp);
+ if (rc)
has_default_mode = 1;
}
@@ -1285,33 +1193,27 @@ static int __devinit install_fb(struct fb_info *info)
fb_videomode_to_var(&info->var, modedb);
}
- pr_debug("xres_virtual %d\n", info->var.xres_virtual);
- pr_debug("bits_per_pixel %d\n", info->var.bits_per_pixel);
-
- pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
- pr_debug("info->fix.line_length = %d\n", info->fix.line_length);
-
if (mfbi->type = MFB_TYPE_OFF)
mfbi->blank = FB_BLANK_NORMAL;
else
mfbi->blank = FB_BLANK_UNBLANK;
if (fsl_diu_check_var(&info->var, info)) {
- printk(KERN_ERR "fb_check_var failed");
+ dev_err(info->dev, "fsl_diu_check_var failed\n");
+ unmap_video_memory(info);
fb_dealloc_cmap(&info->cmap);
return -EINVAL;
}
if (register_framebuffer(info) < 0) {
- printk(KERN_ERR "register_framebuffer failed");
+ dev_err(info->dev, "register_framebuffer failed\n");
unmap_video_memory(info);
fb_dealloc_cmap(&info->cmap);
return -EINVAL;
}
mfbi->registered = 1;
- printk(KERN_INFO "fb%d: %s fb device registered successfully.\n",
- info->node, info->fix.id);
+ dev_info(info->dev, "%s registered successfully\n", mfbi->id);
return 0;
}
@@ -1343,13 +1245,13 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
/* This is the workaround for underrun */
if (status & INT_UNDRUN) {
out_be32(&hw->diu_mode, 0);
- pr_debug("Err: DIU occurs underrun!\n");
udelay(1);
out_be32(&hw->diu_mode, 1);
}
#if defined(CONFIG_NOT_COHERENT_CACHE)
else if (status & INT_VSYNC) {
unsigned int i;
+
for (i = 0; i < coherence_data_size;
i += d_cache_line_size)
__asm__ __volatile__ (
@@ -1364,30 +1266,30 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
static int request_irq_local(int irq)
{
- unsigned long status, ints;
+ u32 ints;
struct diu *hw;
int ret;
hw = dr.diu_reg;
/* Read to clear the status */
- status = in_be32(&hw->int_status);
+ in_be32(&hw->int_status);
- ret = request_irq(irq, fsl_diu_isr, 0, "diu", NULL);
- if (ret)
- pr_info("Request diu IRQ failed.\n");
- else {
+ ret = request_irq(irq, fsl_diu_isr, 0, "fsl-diu-fb", NULL);
+ if (!ret) {
ints = INT_PARERR | INT_LS_BF_VS;
#if !defined(CONFIG_NOT_COHERENT_CACHE)
ints |= INT_VSYNC;
#endif
+
if (dr.mode = MFB_MODE2 || dr.mode = MFB_MODE3)
ints |= INT_VSYNC_WB;
/* Read to clear the status */
- status = in_be32(&hw->int_status);
+ in_be32(&hw->int_status);
out_be32(&hw->int_mask, ints);
}
+
return ret;
}
@@ -1435,34 +1337,31 @@ static int fsl_diu_resume(struct platform_device *ofdev)
static int allocate_buf(struct device *dev, struct diu_addr *buf, u32 size,
u32 bytes_align)
{
- u32 offset, ssize;
- u32 mask;
- dma_addr_t paddr = 0;
+ u32 offset;
+ dma_addr_t mask;
- ssize = size + bytes_align;
- buf->vaddr = dma_alloc_coherent(dev, ssize, &paddr, GFP_DMA |
- __GFP_ZERO);
+ buf->vaddr + dma_alloc_coherent(dev, size + bytes_align, &buf->paddr,
+ GFP_DMA | __GFP_ZERO);
if (!buf->vaddr)
return -ENOMEM;
- buf->paddr = (__u32) paddr;
-
mask = bytes_align - 1;
- offset = (u32)buf->paddr & mask;
+ offset = buf->paddr & mask;
if (offset) {
buf->offset = bytes_align - offset;
- buf->paddr = (u32)buf->paddr + offset;
+ buf->paddr = buf->paddr + offset;
} else
buf->offset = 0;
+
return 0;
}
static void free_buf(struct device *dev, struct diu_addr *buf, u32 size,
u32 bytes_align)
{
- dma_free_coherent(dev, size + bytes_align,
- buf->vaddr, (buf->paddr - buf->offset));
- return;
+ dma_free_coherent(dev, size + bytes_align, buf->vaddr,
+ buf->paddr - buf->offset);
}
static ssize_t store_monitor(struct device *device,
@@ -1506,13 +1405,12 @@ static ssize_t show_monitor(struct device *device,
return 0;
}
-static int __devinit fsl_diu_probe(struct platform_device *ofdev)
+static int __devinit fsl_diu_probe(struct platform_device *pdev)
{
- struct device_node *np = ofdev->dev.of_node;
+ struct device_node *np = pdev->dev.of_node;
struct mfb_info *mfbi;
- phys_addr_t dummy_ad_addr;
+ phys_addr_t dummy_ad_addr = 0;
int ret, i, error = 0;
- struct resource res;
struct fsl_diu_data *machine_data;
int diu_mode;
@@ -1522,9 +1420,9 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++) {
machine_data->fsl_diu_info[i] - framebuffer_alloc(sizeof(struct mfb_info), &ofdev->dev);
+ framebuffer_alloc(sizeof(struct mfb_info), &pdev->dev);
if (!machine_data->fsl_diu_info[i]) {
- dev_err(&ofdev->dev, "cannot allocate memory\n");
+ dev_err(&pdev->dev, "cannot allocate memory\n");
ret = -ENOMEM;
goto error2;
}
@@ -1544,20 +1442,9 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
}
}
- ret = of_address_to_resource(np, 0, &res);
- if (ret) {
- dev_err(&ofdev->dev, "could not obtain DIU address\n");
- goto error;
- }
- if (!res.start) {
- dev_err(&ofdev->dev, "invalid DIU address\n");
- goto error;
- }
- dev_dbg(&ofdev->dev, "%s, res.start: 0x%08x\n", __func__, res.start);
-
- dr.diu_reg = ioremap(res.start, sizeof(struct diu));
+ dr.diu_reg = of_iomap(np, 0);
if (!dr.diu_reg) {
- dev_err(&ofdev->dev, "Err: can't map DIU registers!\n");
+ dev_err(&pdev->dev, "cannot map DIU registers\n");
ret = -EFAULT;
goto error2;
}
@@ -1570,25 +1457,25 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
machine_data->irq = irq_of_parse_and_map(np, 0);
if (!machine_data->irq) {
- dev_err(&ofdev->dev, "could not get DIU IRQ\n");
+ dev_err(&pdev->dev, "could not get DIU IRQ\n");
ret = -EINVAL;
goto error;
}
machine_data->monitor_port = monitor_port;
/* Area descriptor memory pool aligns to 64-bit boundary */
- if (allocate_buf(&ofdev->dev, &pool.ad,
+ if (allocate_buf(&pdev->dev, &pool.ad,
sizeof(struct diu_ad) * FSL_AOI_NUM, 8))
return -ENOMEM;
/* Get memory for Gamma Table - 32-byte aligned memory */
- if (allocate_buf(&ofdev->dev, &pool.gamma, 768, 32)) {
+ if (allocate_buf(&pdev->dev, &pool.gamma, 768, 32)) {
ret = -ENOMEM;
goto error;
}
/* For performance, cursor bitmap buffer aligns to 32-byte boundary */
- if (allocate_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
+ if (allocate_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
32)) {
ret = -ENOMEM;
goto error;
@@ -1630,16 +1517,13 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
mfbi->ad->paddr = pool.ad.paddr + i * sizeof(struct diu_ad);
ret = install_fb(machine_data->fsl_diu_info[i]);
if (ret) {
- dev_err(&ofdev->dev,
- "Failed to register framebuffer %d\n",
- i);
+ dev_err(&pdev->dev, "could not register fb %d\n", i);
goto error;
}
}
if (request_irq_local(machine_data->irq)) {
- dev_err(machine_data->fsl_diu_info[0]->dev,
- "could not request irq for diu.");
+ dev_err(&pdev->dev, "could not claim irq\n");
goto error;
}
@@ -1651,25 +1535,24 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
error = device_create_file(machine_data->fsl_diu_info[0]->dev,
&machine_data->dev_attr);
if (error) {
- dev_err(machine_data->fsl_diu_info[0]->dev,
- "could not create sysfs %s file\n",
+ dev_err(&pdev->dev, "could not create sysfs file %s\n",
machine_data->dev_attr.attr.name);
}
- dev_set_drvdata(&ofdev->dev, machine_data);
+ dev_set_drvdata(&pdev->dev, machine_data);
return 0;
error:
- for (i = ARRAY_SIZE(machine_data->fsl_diu_info);
- i > 0; i--)
- uninstall_fb(machine_data->fsl_diu_info[i - 1]);
+ for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
+ uninstall_fb(machine_data->fsl_diu_info[i]);
+
if (pool.ad.vaddr)
- free_buf(&ofdev->dev, &pool.ad,
+ free_buf(&pdev->dev, &pool.ad,
sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
if (pool.gamma.vaddr)
- free_buf(&ofdev->dev, &pool.gamma, 768, 32);
+ free_buf(&pdev->dev, &pool.gamma, 768, 32);
if (pool.cursor.vaddr)
- free_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
+ free_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
32);
if (machine_data->dummy_aoi_virt)
fsl_diu_free(machine_data->dummy_aoi_virt, 64);
@@ -1684,25 +1567,23 @@ error2:
return ret;
}
-
-static int fsl_diu_remove(struct platform_device *ofdev)
+static int fsl_diu_remove(struct platform_device *pdev)
{
struct fsl_diu_data *machine_data;
int i;
- machine_data = dev_get_drvdata(&ofdev->dev);
+ machine_data = dev_get_drvdata(&pdev->dev);
disable_lcdc(machine_data->fsl_diu_info[0]);
free_irq_local(machine_data->irq);
- for (i = ARRAY_SIZE(machine_data->fsl_diu_info); i > 0; i--)
- uninstall_fb(machine_data->fsl_diu_info[i - 1]);
+ for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
+ uninstall_fb(machine_data->fsl_diu_info[i]);
if (pool.ad.vaddr)
- free_buf(&ofdev->dev, &pool.ad,
+ free_buf(&pdev->dev, &pool.ad,
sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
if (pool.gamma.vaddr)
- free_buf(&ofdev->dev, &pool.gamma, 768, 32);
+ free_buf(&pdev->dev, &pool.gamma, 768, 32);
if (pool.cursor.vaddr)
- free_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
- 32);
+ free_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2, 32);
if (machine_data->dummy_aoi_virt)
fsl_diu_free(machine_data->dummy_aoi_virt, 64);
iounmap(dr.diu_reg);
@@ -1754,7 +1635,7 @@ MODULE_DEVICE_TABLE(of, fsl_diu_match);
static struct platform_driver fsl_diu_driver = {
.driver = {
- .name = "fsl_diu",
+ .name = "fsl-diu-fb",
.owner = THIS_MODULE,
.of_match_table = fsl_diu_match,
},
@@ -1783,43 +1664,48 @@ static int __init fsl_diu_init(void)
#else
monitor_port = fsl_diu_name_to_port(monitor_string);
#endif
- printk(KERN_INFO "Freescale DIU driver\n");
+ pr_info("Freescale Display Interface Unit (DIU) framebuffer driver\n");
#ifdef CONFIG_NOT_COHERENT_CACHE
np = of_find_node_by_type(NULL, "cpu");
if (!np) {
- printk(KERN_ERR "Err: can't find device node 'cpu'\n");
+ pr_err("fsl-diu-fb: can't find 'cpu' device node\n");
return -ENODEV;
}
prop = of_get_property(np, "d-cache-size", NULL);
if (prop = NULL) {
+ pr_err("fsl-diu-fb: missing 'd-cache-size' property' "
+ "in 'cpu' node\n");
of_node_put(np);
return -ENODEV;
}
- /* Freescale PLRU requires 13/8 times the cache size to do a proper
- displacement flush
+ /*
+ * Freescale PLRU requires 13/8 times the cache size to do a proper
+ * displacement flush
*/
- coherence_data_size = *prop * 13;
+ coherence_data_size = be32_to_cpup(prop) * 13;
coherence_data_size /= 8;
prop = of_get_property(np, "d-cache-line-size", NULL);
if (prop = NULL) {
+ pr_err("fsl-diu-fb: missing 'd-cache-line-size' property' "
+ "in 'cpu' node\n");
of_node_put(np);
return -ENODEV;
}
- d_cache_line_size = *prop;
+ d_cache_line_size = be32_to_cpup(prop);
of_node_put(np);
coherence_data = vmalloc(coherence_data_size);
if (!coherence_data)
return -ENOMEM;
#endif
+
ret = platform_driver_register(&fsl_diu_driver);
if (ret) {
- printk(KERN_ERR
- "fsl-diu: failed to register platform driver\n");
+ pr_err("fsl-diu-fb: failed to register platform driver\n");
#if defined(CONFIG_NOT_COHERENT_CACHE)
vfree(coherence_data);
#endif
@@ -1847,7 +1733,7 @@ module_param_named(mode, fb_mode, charp, 0);
MODULE_PARM_DESC(mode,
"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
module_param_named(bpp, default_bpp, ulong, 0);
-MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified mode");
+MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified in \"mode\"");
module_param_named(monitor, monitor_string, charp, 0);
MODULE_PARM_DESC(monitor, "Specify the monitor port "
"(\"dvi\", \"lvds\", or \"dlvds\") if supported by the platform");
diff --git a/include/linux/fsl-diu-fb.h b/include/linux/fsl-diu-fb.h
index daa9952..df23f59 100644
--- a/include/linux/fsl-diu-fb.h
+++ b/include/linux/fsl-diu-fb.h
@@ -20,18 +20,8 @@
#ifndef __FSL_DIU_FB_H__
#define __FSL_DIU_FB_H__
-/* Arbitrary threshold to determine the allocation method
- * See mpc8610fb_set_par(), map_video_memory(), and unmap_video_memory()
- */
-#define MEM_ALLOC_THRESHOLD (1024*768*4+32)
-
#include <linux/types.h>
-struct mfb_alpha {
- int enable;
- int alpha;
-};
-
struct mfb_chroma_key {
int enable;
__u8 red_max;
@@ -167,7 +157,7 @@ struct diu_hw {
};
struct diu_addr {
- __u8 __iomem *vaddr; /* Virtual address */
+ void *vaddr; /* Virtual address */
dma_addr_t paddr; /* Physical address */
__u32 offset;
};
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14 5:46 UTC (permalink / raw)
To: Rob Clark; +Cc: Tomi Valkeinen, linux-omap, linux-fbdev, archit
In-Reply-To: <CAF6AEGuhjZgH9zFuigjfvHGM9r7jSLY2TTCD0LjZbT+Ubrb12A@mail.gmail.com>
Hi,
On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>>> +{
>>> + int r;
>>> +
>>> + void __iomem *base = hdmi_core_sys_base(ip_data);
>>> +
>>> + /* HPD */
>>> + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>>> +
>>> + return r = 1;
>>> +}
>>> +
>> For HPD the probe should also be on the core interrupt first , and the
>> detect should be dynamic, ie based on the cable connect and disconnect
>> event.So this approach for HPD is not really the way.
>> Also that should be based on the GPIO(63) , I am planning to push a
>> patch on that shortly.
>
>
> Fwiw, we do still need a dssdrv->detect() function from omapdrm
> driver.. if there is another way to implement that function, such as
> with a GPIO, that is great. But somehow or another we need the detect
> function. The implementation can always change later.
Yes we still need a detect , but the implementation would be different
, from the prior experience with the Hot-plug detection it wad found
that the interrupt based way to handle HPD was not the best ,but if
this is just to poll the status then it should be fine.
>
> BR,
> -R
>
Thanks and regards,
Mythri.
^ permalink raw reply
* Re: [REPOST][PATCH 2/2] video: miscellaneous minor changes to the
From: Tormod Volden @ 2011-09-14 6:54 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1315929909-26311-2-git-send-email-timur@freescale.com>
On Tue, Sep 13, 2011 at 6:05 PM, Timur Tabi <timur@freescale.com> wrote:
> Make several minor, miscellaneous changes to the Freescale DIU framebuffer
> driver. These changes "lighten" the code by removing crud, fixing small
> bugs, and fixing some coding style problems. These changes will make it
> easier to make more substantial fixes in the future.
It would be much easier to review this if it is split up into several
commits. At least have the whitespace fixes in a separate commit, and
also the actual bug fixes. "git add -p" is your friend.
> 1. Fix incorrect indentation and spacing with some code.
> 2. Remove debug printks (they don't actually help in debugging the code).
> 3. Clean up some other printks (e.g. use pr_xxx, clean up the text, etc).
> 4. Remove the "default" videomode object since it's just a dupe of the
> first element in the videomode array.
> 5. Remove some superfluous local variables.
> 6. Rename ofdev to pdev, since it's a platform device not an OF device.
> 7. Fix some device tree operations.
> 8. Fix some build warnings.
> 9. Removed some unused structures from the header file.
> 10. Other minor bug fixes and changes.
I would have found natural to split it up into commits like for
example: 1, 2+3, 4, 5+8+9, 10.
> @@ -217,59 +201,59 @@ struct mfb_info {
> int x_aoi_d; /* aoi display x offset to physical screen */
> int y_aoi_d; /* aoi display y offset to physical screen */
> struct fsl_diu_data *parent;
> - u8 *edid_data;
> + void *edid_data;
> };
Why do you convert edid_data from pointer to u8 to pointer to void?
Regards,
Tormod
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14 7:14 UTC (permalink / raw)
To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B87w6Hir-0+SNnckQaDcO=ixgq-U8pFo2U42dwQgtUrZQ@mail.gmail.com>
On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
> Hi,
>
> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> >>> +{
> >>> + int r;
> >>> +
> >>> + void __iomem *base = hdmi_core_sys_base(ip_data);
> >>> +
> >>> + /* HPD */
> >>> + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> >>> +
> >>> + return r = 1;
> >>> +}
> >>> +
> >> For HPD the probe should also be on the core interrupt first , and the
> >> detect should be dynamic, ie based on the cable connect and disconnect
> >> event.So this approach for HPD is not really the way.
> >> Also that should be based on the GPIO(63) , I am planning to push a
> >> patch on that shortly.
> >
> >
> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
> > driver.. if there is another way to implement that function, such as
> > with a GPIO, that is great. But somehow or another we need the detect
> > function. The implementation can always change later.
> Yes we still need a detect , but the implementation would be different
> , from the prior experience with the Hot-plug detection it wad found
> that the interrupt based way to handle HPD was not the best ,but if
> this is just to poll the status then it should be fine.
I'm not sure I understood. First you say the implementation should be
different, but then you say this should be fine. So is this a valid
implementation for detect() or is there a better way to do it?
Tomi
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14 8:34 UTC (permalink / raw)
To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9uDj4kCowbEKgX1J74zWvBarp64Cn4YVom-a8BYtab4w@mail.gmail.com>
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:
> > On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
> >> Hi,
> >>
> >> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
> >> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
> >> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> >> >>> +{
> >> >>> + int r;
> >> >>> +
> >> >>> + void __iomem *base = hdmi_core_sys_base(ip_data);
> >> >>> +
> >> >>> + /* HPD */
> >> >>> + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> >> >>> +
> >> >>> + return r = 1;
> >> >>> +}
> >> >>> +
> >> >> For HPD the probe should also be on the core interrupt first , and the
> >> >> detect should be dynamic, ie based on the cable connect and disconnect
> >> >> event.So this approach for HPD is not really the way.
> >> >> Also that should be based on the GPIO(63) , I am planning to push a
> >> >> patch on that shortly.
> >> >
> >> >
> >> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
> >> > driver.. if there is another way to implement that function, such as
> >> > with a GPIO, that is great. But somehow or another we need the detect
> >> > function. The implementation can always change later.
> >> Yes we still need a detect , but the implementation would be different
> >> , from the prior experience with the Hot-plug detection it wad found
> >> that the interrupt based way to handle HPD was not the best ,but if
> >> this is just to poll the status then it should be fine.
> >
> > I'm not sure I understood. First you say the implementation should be
> > different, but then you say this should be fine. So is this a valid
> > implementation for detect() or is there a better way to do it?
> >
> There is a better way to handle Hot-plug detection and notification..
> But depends on what is the purpose of this function, Ideally a detect
The purpose of the detect function is to return true or false, depending
on whether a (preferably powered-on) monitor is connecter via a cable or
not. So it tells if there's a display that can be used or not.
> would be the case to dynamically detect whether the cable is connected
> on not , But all this function does is to see the state of the HPD bit
> in core state statically.
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.
> 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?
Tomi
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14 8:39 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1315984468.2172.10.camel@deskari>
On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
>> Hi,
>>
>> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
>> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> >>> +{
>> >>> + int r;
>> >>> +
>> >>> + void __iomem *base = hdmi_core_sys_base(ip_data);
>> >>> +
>> >>> + /* HPD */
>> >>> + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>> >>> +
>> >>> + return r = 1;
>> >>> +}
>> >>> +
>> >> For HPD the probe should also be on the core interrupt first , and the
>> >> detect should be dynamic, ie based on the cable connect and disconnect
>> >> event.So this approach for HPD is not really the way.
>> >> Also that should be based on the GPIO(63) , I am planning to push a
>> >> patch on that shortly.
>> >
>> >
>> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
>> > driver.. if there is another way to implement that function, such as
>> > with a GPIO, that is great. But somehow or another we need the detect
>> > function. The implementation can always change later.
>> Yes we still need a detect , but the implementation would be different
>> , from the prior experience with the Hot-plug detection it wad found
>> that the interrupt based way to handle HPD was not the best ,but if
>> this is just to poll the status then it should be fine.
>
> I'm not sure I understood. First you say the implementation should be
> different, but then you say this should be fine. So is this a valid
> implementation for detect() or is there a better way to do it?
>
There is a better way to handle Hot-plug detection and notification..
But depends on what is the purpose of this function, Ideally a detect
would be the case to dynamically detect whether the cable is connected
on not , But all this function does is to see the state of the HPD bit
in core state statically.
So I said if the purpose of this function is only to check for the HPD
state bit it is fine.
Thanks and regards,
Mythri.
> Tomi
>
>
>
>
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14 8:48 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1315989282.2172.36.camel@deskari>
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:
>> > On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
>> >> Hi,
>> >>
>> >> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
>> >> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>> >> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> >> >>> +{
>> >> >>> + int r;
>> >> >>> +
>> >> >>> + void __iomem *base = hdmi_core_sys_base(ip_data);
>> >> >>> +
>> >> >>> + /* HPD */
>> >> >>> + r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>> >> >>> +
>> >> >>> + return r = 1;
>> >> >>> +}
>> >> >>> +
>> >> >> For HPD the probe should also be on the core interrupt first , and the
>> >> >> detect should be dynamic, ie based on the cable connect and disconnect
>> >> >> event.So this approach for HPD is not really the way.
>> >> >> Also that should be based on the GPIO(63) , I am planning to push a
>> >> >> patch on that shortly.
>> >> >
>> >> >
>> >> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
>> >> > driver.. if there is another way to implement that function, such as
>> >> > with a GPIO, that is great. But somehow or another we need the detect
>> >> > function. The implementation can always change later.
>> >> Yes we still need a detect , but the implementation would be different
>> >> , from the prior experience with the Hot-plug detection it wad found
>> >> that the interrupt based way to handle HPD was not the best ,but if
>> >> this is just to poll the status then it should be fine.
>> >
>> > I'm not sure I understood. First you say the implementation should be
>> > different, but then you say this should be fine. So is this a valid
>> > implementation for detect() or is there a better way to do it?
>> >
>> There is a better way to handle Hot-plug detection and notification..
>> But depends on what is the purpose of this function, Ideally a detect
>
> The purpose of the detect function is to return true or false, depending
> on whether a (preferably powered-on) monitor is connecter via a cable or
> not. So it tells if there's a display that can be used or not.
>
>> would be the case to dynamically detect whether the cable is connected
>> on not , But all this function does is to see the state of the HPD bit
>> in core state statically.
>
> 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?
>
>> 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
ready to be read, But this is a static check that is done in this
function.
Thanks and regards,
Mythri.
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14 8:57 UTC (permalink / raw)
To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B-GWN+oJhcVbB24swfmH=-dE4sViNqF+hX6=D1owzOLzA@mail.gmail.com>
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.
Tomi
^ permalink raw reply
* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14 12:32 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1315990630.2172.51.camel@deskari>
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.
Thanks and regards,
Mythri.
> Tomi
>
>
>
^ permalink raw reply
* 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
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;
as well as URLs for NNTP newsgroup(s).