* [PATCH 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-08 11:50 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <19a46cc15dc7add0671dd8c32f5398249c0420ab.1343912533.git.cmahapatra@ti.com>
All the cpu_is checks have been moved to dss_init_features function providing a
much more generic and cleaner interface. The OMAP version and revision specific
functions are initialized by dss_features structure local to dss.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dss.c | 154 ++++++++++++++++++++++++++++++-----------
1 file changed, 114 insertions(+), 40 deletions(-)
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7b1c6ac..f5971ac 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -65,6 +65,20 @@ struct dss_reg {
static int dss_runtime_get(void);
static void dss_runtime_put(void);
+static bool check_dss_cinfo_fck(void);
+static bool check_dss_cinfo_fck_34xx(void);
+
+static int dss_get_clk_24xx(struct clk *clk);
+static int dss_get_clk_3xxx(struct clk *clk);
+static int dss_get_clk_44xx(struct clk *clk);
+
+struct dss_features {
+ u16 fck_div_max;
+ int factor;
+ bool (*check_cinfo_fck) (void);
+ int (*get_clk) (struct clk *clk);
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -83,6 +97,8 @@ static struct {
bool ctx_valid;
u32 ctx[DSS_SZ_REGS / sizeof(u32)];
+
+ const struct dss_features *feat;
} dss;
static const char * const dss_generic_clk_source_names[] = {
@@ -91,6 +107,34 @@ static const char * const dss_generic_clk_source_names[] = {
[OMAP_DSS_CLK_SRC_FCK] = "DSS_FCK",
};
+static const struct dss_features omap2_dss_features = {
+ .fck_div_max = 16,
+ .factor = 2,
+ .check_cinfo_fck = check_dss_cinfo_fck,
+ .get_clk = dss_get_clk_24xx,
+};
+
+static const struct dss_features omap34_dss_features = {
+ .fck_div_max = 16,
+ .factor = 2,
+ .check_cinfo_fck = check_dss_cinfo_fck_34xx,
+ .get_clk = dss_get_clk_3xxx,
+};
+
+static const struct dss_features omap36_dss_features = {
+ .fck_div_max = 32,
+ .factor = 1,
+ .check_cinfo_fck = check_dss_cinfo_fck,
+ .get_clk = dss_get_clk_3xxx,
+};
+
+static const struct dss_features omap4_dss_features = {
+ .fck_div_max = 32,
+ .factor = 1,
+ .check_cinfo_fck = check_dss_cinfo_fck,
+ .get_clk = dss_get_clk_44xx,
+};
+
static inline void dss_write_reg(const struct dss_reg idx, u32 val)
{
__raw_writel(val, dss.base + idx.idx);
@@ -236,7 +280,6 @@ const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src)
return dss_generic_clk_source_names[clk_src];
}
-
void dss_dump_clocks(struct seq_file *s)
{
unsigned long dpll4_ck_rate;
@@ -259,18 +302,10 @@ void dss_dump_clocks(struct seq_file *s)
seq_printf(s, "dpll4_ck %lu\n", dpll4_ck_rate);
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- seq_printf(s, "%s (%s) = %lu / %lu = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
- else
- seq_printf(s, "%s (%s) = %lu / %lu * 2 = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
+ seq_printf(s, "%s (%s) = %lu / %lu * %d = %lu\n",
+ fclk_name, fclk_real_name, dpll4_ck_rate,
+ dpll4_ck_rate / dpll4_m4_ck_rate,
+ dss.feat->factor, fclk_rate);
} else {
seq_printf(s, "%s (%s) = %lu\n",
fclk_name, fclk_real_name,
@@ -461,6 +496,25 @@ unsigned long dss_get_dpll4_rate(void)
return 0;
}
+static bool check_dss_cinfo_fck_34xx(void)
+{
+ unsigned long prate = dss_get_dpll4_rate();
+ unsigned long fck = clk_get_rate(dss.dss_clk);
+
+ if (prate = dss.cache_prate || dss.cache_dss_cinfo.fck = fck)
+ return true;
+ return false;
+}
+
+static bool check_dss_cinfo_fck(void)
+{
+ unsigned long fck = clk_get_rate(dss.dss_clk);
+
+ if (dss.cache_dss_cinfo.fck = fck)
+ return true;
+ return false;
+}
+
int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
struct dispc_clock_info *dispc_cinfo)
{
@@ -470,7 +524,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
unsigned long fck, max_dss_fck;
- u16 fck_div, fck_div_max = 16;
+ u16 fck_div;
int match = 0;
int min_fck_per_pck;
@@ -479,10 +533,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
- fck = clk_get_rate(dss.dss_clk);
- if (req_pck = dss.cache_req_pck &&
- ((cpu_is_omap34xx() && prate = dss.cache_prate) ||
- dss.cache_dss_cinfo.fck = fck)) {
+ if (req_pck = dss.cache_req_pck && dss.feat->check_cinfo_fck()) {
DSSDBG("dispc clock info found from cache.\n");
*dss_cinfo = dss.cache_dss_cinfo;
*dispc_cinfo = dss.cache_dispc_cinfo;
@@ -519,13 +570,10 @@ retry:
goto found;
} else {
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- fck_div_max = 32;
-
- for (fck_div = fck_div_max; fck_div > 0; --fck_div) {
+ for (fck_div = dss.feat->fck_div_max; fck_div > 0; --fck_div) {
struct dispc_clock_info cur_dispc;
- if (fck_div_max = 32)
+ if (dss.feat->fck_div_max = 32)
fck = prate / fck_div;
else
fck = prate / fck_div * 2;
@@ -619,6 +667,32 @@ enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void)
return REG_GET(DSS_CONTROL, 15, 15);
}
+static int dss_get_clk_24xx(struct clk *clk)
+{
+ clk = NULL;
+ return true;
+}
+
+static int dss_get_clk_3xxx(struct clk *clk)
+{
+ clk = clk_get(NULL, "dpll4_m4_ck");
+ if (IS_ERR(clk)) {
+ DSSERR("Failed to get dpll4_m4_ck\n");
+ return PTR_ERR(clk);
+ }
+ return true;
+}
+
+static int dss_get_clk_44xx(struct clk *clk)
+{
+ clk = clk_get(NULL, "dpll_per_m5x2_ck");
+ if (IS_ERR(clk)) {
+ DSSERR("Failed to get dpll_per_m5x2_ck\n");
+ return PTR_ERR(clk);
+ }
+ return true;
+}
+
static int dss_get_clocks(void)
{
struct clk *clk;
@@ -633,23 +707,9 @@ static int dss_get_clocks(void)
dss.dss_clk = clk;
- if (cpu_is_omap34xx()) {
- clk = clk_get(NULL, "dpll4_m4_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll4_m4_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else if (cpu_is_omap44xx()) {
- clk = clk_get(NULL, "dpll_per_m5x2_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll_per_m5x2_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else { /* omap24xx */
- clk = NULL;
- }
+ r = dss.feat->get_clk(clk);
+ if (r != true)
+ goto err;
dss.dpll4_m4_ck = clk;
@@ -704,6 +764,18 @@ void dss_debug_dump_clocks(struct seq_file *s)
}
#endif
+static void dss_init_features(void)
+{
+ if (cpu_is_omap24xx())
+ dss.feat = &omap2_dss_features;
+ else if (cpu_is_omap34xx())
+ dss.feat = &omap34_dss_features;
+ else if (cpu_is_omap3630())
+ dss.feat = &omap36_dss_features;
+ else
+ dss.feat = &omap4_dss_features;
+}
+
/* DSS HW IP initialisation */
static int __init omap_dsshw_probe(struct platform_device *pdev)
{
@@ -750,6 +822,8 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
dss.lcd_clk_source[0] = OMAP_DSS_CLK_SRC_FCK;
dss.lcd_clk_source[1] = OMAP_DSS_CLK_SRC_FCK;
+ dss_init_features();
+
rev = dss_read_reg(DSS_REVISION);
printk(KERN_INFO "OMAP DSS rev %d.%d\n",
FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
--
1.7.10
^ permalink raw reply related
* [PATCH 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-08 11:49 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <5344e530a125ef5c5dfeb00e54b7d32df6169aa9.1343912532.git.cmahapatra@ti.com>
All the cpu_is checks have been moved to dispc_init_features function providing
a much more generic and cleaner interface. The OMAP version and revision
specific functions are initialized by dispc_features structure local to dispc.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dispc.c | 476 ++++++++++++++++++++++++++-------------
1 file changed, 315 insertions(+), 161 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5b289c5..7e0b080 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -75,12 +75,60 @@ enum omap_burst_size {
#define REG_FLD_MOD(idx, val, start, end) \
dispc_write_reg(idx, FLD_MOD(dispc_read_reg(idx), val, start, end))
+static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+ u16 out_width, u16 out_height, enum omap_color_mode color_mode,
+ bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+ int *decim_y, u16 pos_x, unsigned long *core_clk);
+static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+ u16 out_width, u16 out_height, enum omap_color_mode color_mode,
+ bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+ int *decim_y, u16 pos_x, unsigned long *core_clk);
+static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings, u16 width, u16 height,
+ u16 out_width, u16 out_height, enum omap_color_mode color_mode,
+ bool *five_taps, int *x_predecim, int *y_predecim, int *decim_x,
+ int *decim_y, u16 pos_x, unsigned long *core_clk);
+
+static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height);
+static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height);
+static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height);
+
+static bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp);
+static bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp);
+
+static void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp);
+static void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp);
+
struct dispc_irq_stats {
unsigned long last_reset;
unsigned irq_count;
unsigned irqs[32];
};
+struct dispc_features {
+ int (*calc_scaling) (enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk);
+ unsigned long (*calc_core_clk) (enum omap_channel channel,
+ u16 width, u16 height, u16 out_width, u16 out_height);
+ bool (*lcd_timings_ok) (int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp);
+ void (*set_lcd_timings_hv) (enum omap_channel channel, int hsw, int hfp,
+ int hbp, int vsw, int vfp, int vbp);
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -101,6 +149,8 @@ static struct {
bool ctx_valid;
u32 ctx[DISPC_SZ_REGS / sizeof(u32)];
+ const struct dispc_features *feat;
+
#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
spinlock_t irq_stats_lock;
struct dispc_irq_stats irq_stats;
@@ -210,6 +260,34 @@ static const struct {
},
};
+static const struct dispc_features omap2_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_24xx,
+ .calc_core_clk = calc_core_clk_24xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_24xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_24xx,
+};
+
+static const struct dispc_features omap3_2_1_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_24xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_24xx,
+};
+
+static const struct dispc_features omap3_3_0_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_44xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_44xx,
+};
+
+static const struct dispc_features omap4_dispc_features = {
+ .calc_scaling = dispc_ovl_calc_scaling_44xx,
+ .calc_core_clk = calc_core_clk_44xx,
+ .lcd_timings_ok = _dispc_lcd_timings_ok_44xx,
+ .set_lcd_timings_hv = _dispc_mgr_set_lcd_timings_hv_44xx,
+};
+
static void _omap_dispc_set_irqs(void);
static inline void dispc_write_reg(const u16 idx, u32 val)
@@ -1939,7 +2017,18 @@ static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
return core_clk;
}
-static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
+static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (height > out_height && width > out_width)
+ return pclk * 4;
+ else
+ return pclk * 2;
+}
+
+static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
u16 height, u16 out_width, u16 out_height)
{
unsigned int hf, vf;
@@ -1958,25 +2047,163 @@ static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
hf = 2;
else
hf = 1;
-
if (height > out_height)
vf = 2;
else
vf = 1;
- if (cpu_is_omap24xx()) {
- if (vf > 1 && hf > 1)
- return pclk * 4;
- else
- return pclk * 2;
- } else if (cpu_is_omap34xx()) {
- return pclk * vf * hf;
- } else {
- if (hf > 1)
- return DIV_ROUND_UP(pclk, out_width) * width;
- else
- return pclk;
+ return pclk * vf * hf;
+}
+
+static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (width > out_width)
+ return DIV_ROUND_UP(pclk, out_width) * width;
+ else
+ return pclk;
+}
+
+static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+ *five_taps = false;
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+ error = (in_width > maxsinglelinewidth || !*core_clk ||
+ *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale max input width exceeded");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = calc_core_clk_five_taps(channel, mgr_timings,
+ in_width, in_height, out_width, out_height, color_mode);
+
+ error = check_horiz_timing_omap3(channel, mgr_timings, pos_x,
+ in_width, in_height, out_width, out_height);
+
+ if (in_width > maxsinglelinewidth)
+ if (in_height > out_height &&
+ in_height < out_height * 2)
+ *five_taps = false;
+ if (!*five_taps)
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+
+ error = (error || in_width > maxsinglelinewidth * 2 ||
+ (in_width > maxsinglelinewidth && *five_taps) ||
+ !*core_clk || *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width, height,
+ out_width, out_height)){
+ DSSERR("horizontal timing too tight\n");
+ return -EINVAL;
+ }
+
+ if (in_width > (maxsinglelinewidth * 2)) {
+ DSSERR("Cannot setup scaling");
+ DSSERR("width exceeds maximum width possible");
+ return -EINVAL;
+ }
+
+ if (in_width > maxsinglelinewidth && *five_taps) {
+ DSSERR("cannot setup scaling with five taps");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ u16 in_width, in_width_max;
+ int decim_x_min = *decim_x;
+ u16 in_height = DIV_ROUND_UP(height, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ in_width_max = dispc_core_clk_rate() /
+ DIV_ROUND_UP(dispc_mgr_pclk_rate(channel), out_width);
+ *decim_x = DIV_ROUND_UP(width, in_width_max);
+
+ *decim_x = *decim_x > decim_x_min ? *decim_x : decim_x_min;
+ if (*decim_x > *x_predecim)
+ return -EINVAL;
+
+ do {
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ } while (*decim_x <= *x_predecim &&
+ in_width > maxsinglelinewidth && ++*decim_x);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale width exceeds max line width");
+ return -EINVAL;
}
+
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width, in_height,
+ out_width, out_height);
+ return 0;
}
static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1988,12 +2215,9 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
- const int maxsinglelinewidth - dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
const int max_decim_limit = 16;
unsigned long core_clk = 0;
- int decim_x, decim_y, error, min_factor;
- u16 in_width, in_height, in_width_max = 0;
+ int decim_x, decim_y, ret;
if (width = out_width && height = out_height)
return 0;
@@ -2017,118 +2241,17 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
- min_factor = min(decim_x, decim_y);
-
if (decim_x > *x_predecim || out_width > width * 8)
return -EINVAL;
if (decim_y > *y_predecim || out_height > height * 8)
return -EINVAL;
- if (cpu_is_omap24xx()) {
- *five_taps = false;
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- error = (in_width > maxsinglelinewidth || !core_clk ||
- core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
- error);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale max input width exceeded");
- return -EINVAL;
- }
- } else if (cpu_is_omap34xx()) {
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk_five_taps(channel, mgr_timings,
- in_width, in_height, out_width, out_height,
- color_mode);
-
- error = check_horiz_timing_omap3(channel, mgr_timings,
- pos_x, in_width, in_height, out_width,
- out_height);
-
- if (in_width > maxsinglelinewidth)
- if (in_height > out_height &&
- in_height < out_height * 2)
- *five_taps = false;
- if (!*five_taps)
- core_clk = calc_core_clk(channel, in_width,
- in_height, out_width, out_height);
- error = (error || in_width > maxsinglelinewidth * 2 ||
- (in_width > maxsinglelinewidth && *five_taps) ||
- !core_clk || core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim
- && error);
-
- if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width,
- height, out_width, out_height)){
- DSSERR("horizontal timing too tight\n");
- return -EINVAL;
- }
-
- if (in_width > (maxsinglelinewidth * 2)) {
- DSSERR("Cannot setup scaling");
- DSSERR("width exceeds maximum width possible");
- return -EINVAL;
- }
-
- if (in_width > maxsinglelinewidth && *five_taps) {
- DSSERR("cannot setup scaling with five taps");
- return -EINVAL;
- }
- } else {
- int decim_x_min = decim_x;
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width_max = dispc_core_clk_rate() /
- DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
- out_width);
- decim_x = DIV_ROUND_UP(width, in_width_max);
-
- decim_x = decim_x > decim_x_min ? decim_x : decim_x_min;
- if (decim_x > *x_predecim)
- return -EINVAL;
-
- do {
- in_width = DIV_ROUND_UP(width, decim_x);
- } while (decim_x <= *x_predecim &&
- in_width > maxsinglelinewidth && decim_x++);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale width exceeds max line width");
- return -EINVAL;
- }
-
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- }
+ ret = dispc.feat->calc_scaling(channel, mgr_timings, width, height,
+ out_width, out_height, color_mode, five_taps, x_predecim,
+ y_predecim, &decim_x, &decim_y, pos_x, &core_clk);
+ if (ret)
+ return ret;
DSSDBG("required core clk rate = %lu Hz\n", core_clk);
DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
@@ -2601,27 +2724,28 @@ static bool _dispc_mgr_size_ok(u16 width, u16 height)
height <= dss_feat_get_param_max(FEAT_PARAM_MGR_HEIGHT);
}
-static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
+static bool _dispc_lcd_timings_ok_24xx(int hsw, int hfp, int hbp,
int vsw, int vfp, int vbp)
{
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- if (hsw < 1 || hsw > 64 ||
- hfp < 1 || hfp > 256 ||
- hbp < 1 || hbp > 256 ||
- vsw < 1 || vsw > 64 ||
- vfp < 0 || vfp > 255 ||
- vbp < 0 || vbp > 255)
- return false;
- } else {
- if (hsw < 1 || hsw > 256 ||
- hfp < 1 || hfp > 4096 ||
- hbp < 1 || hbp > 4096 ||
- vsw < 1 || vsw > 256 ||
- vfp < 0 || vfp > 4095 ||
- vbp < 0 || vbp > 4095)
- return false;
- }
-
+ if (hsw < 1 || hsw > 64 ||
+ hfp < 1 || hfp > 256 ||
+ hbp < 1 || hbp > 256 ||
+ vsw < 1 || vsw > 64 ||
+ vfp < 0 || vfp > 255 ||
+ vbp < 0 || vbp > 255)
+ return false;
+ return true;
+}
+static bool _dispc_lcd_timings_ok_44xx(int hsw, int hfp, int hbp,
+ int vsw, int vfp, int vbp)
+{
+ if (hsw < 1 || hsw > 256 ||
+ hfp < 1 || hfp > 4096 ||
+ hbp < 1 || hbp > 4096 ||
+ vsw < 1 || vsw > 256 ||
+ vfp < 0 || vfp > 4095 ||
+ vbp < 0 || vbp > 4095)
+ return false;
return true;
}
@@ -2633,7 +2757,8 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
timings_ok = _dispc_mgr_size_ok(timings->x_res, timings->y_res);
if (dss_mgr_is_lcd(channel))
- timings_ok = timings_ok && _dispc_lcd_timings_ok(timings->hsw,
+ timings_ok = timings_ok &&
+ dispc.feat->lcd_timings_ok(timings->hsw,
timings->hfp, timings->hbp,
timings->vsw, timings->vfp,
timings->vbp);
@@ -2641,6 +2766,34 @@ bool dispc_mgr_timings_ok(enum omap_channel channel,
return timings_ok;
}
+static void _dispc_mgr_set_lcd_timings_hv_24xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp)
+{
+ u32 timing_h, timing_v;
+
+ timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
+ FLD_VAL(hbp-1, 27, 20);
+ timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
+ FLD_VAL(vbp, 27, 20);
+
+ dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
+ dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+}
+
+static void _dispc_mgr_set_lcd_timings_hv_44xx(enum omap_channel channel,
+ int hsw, int hfp, int hbp, int vsw, int vfp, int vbp)
+{
+ u32 timing_h, timing_v;
+
+ timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
+ FLD_VAL(hbp-1, 31, 20);
+ timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
+ FLD_VAL(vbp, 31, 20);
+
+ dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
+ dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+}
+
static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
int hfp, int hbp, int vsw, int vfp, int vbp,
enum omap_dss_signal_level vsync_level,
@@ -2650,25 +2803,10 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
enum omap_dss_signal_edge sync_pclk_edge)
{
- u32 timing_h, timing_v, l;
+ u32 l;
bool onoff, rf, ipc;
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
- FLD_VAL(hbp-1, 27, 20);
-
- timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
- FLD_VAL(vbp, 27, 20);
- } else {
- timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
- FLD_VAL(hbp-1, 31, 20);
-
- timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
- FLD_VAL(vbp, 31, 20);
- }
-
- dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
- dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
+ dispc.feat->set_lcd_timings_hv(channel, hsw, hfp, hbp, vsw, vfp, vbp);
switch (data_pclk_edge) {
case OMAPDSS_DRIVE_SIG_RISING_EDGE:
@@ -3671,6 +3809,20 @@ static void _omap_dispc_initial_config(void)
dispc_ovl_enable_zorder_planes();
}
+static void dispc_init_features(void)
+{
+ if (cpu_is_omap24xx()) {
+ dispc.feat = &omap2_dispc_features;
+ } else if (cpu_is_omap34xx()) {
+ if (omap_rev() < OMAP3430_REV_ES3_0)
+ dispc.feat = &omap3_2_1_dispc_features;
+ else
+ dispc.feat = &omap3_3_0_dispc_features;
+ } else {
+ dispc.feat = &omap4_dispc_features;
+ }
+}
+
/* DISPC HW IP initialisation */
static int __init omap_dispchw_probe(struct platform_device *pdev)
{
@@ -3725,6 +3877,8 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
dispc.dss_clk = clk;
+ dispc_init_features();
+
pm_runtime_enable(&pdev->dev);
r = dispc_runtime_get();
--
1.7.10
^ permalink raw reply related
* Re: [PATCH 1/2] ARM: pgtable: add pgprot_writethrough() macro
From: Grazvydas Ignotas @ 2012-08-08 11:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1337716444-30139-2-git-send-email-siarhei.siamashka@gmail.com>
On Tue, May 22, 2012 at 10:54 PM, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> Needed for remapping pages with write-through cacheable
> attribute. May be useful for framebuffers.
With this series applied, some of my framebuffer programs get over 70%
performance improvement, could we get this patch in? OMAP DSS
maintainer agreed to take the second patch if this can be merged.
> Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> ---
> arch/arm/include/asm/pgtable.h | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index f66626d..04297fa 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -103,6 +103,9 @@ extern pgprot_t pgprot_kernel;
> #define pgprot_stronglyordered(prot) \
> __pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_UNCACHED)
>
> +#define pgprot_writethrough(prot) \
> + __pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_WRITETHROUGH)
> +
> #ifdef CONFIG_ARM_DMA_MEM_BUFFERABLE
> #define pgprot_dmacoherent(prot) \
> __pgprot_modify(prot, L_PTE_MT_MASK, L_PTE_MT_BUFFERABLE | L_PTE_XN)
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gražvydas
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 9:36 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344415685.4932.14.camel@deskari>
On Wednesday 08 August 2012 02:18 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 14:08 +0530, Archit Taneja wrote:
>> On Wednesday 08 August 2012 01:43 PM, Tomi Valkeinen wrote:
>>> On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
>>>
>>>> Okay, one thing which I want to align on is that most of these functions
>>>> don't really do the actual configurations. That is, they'll just update
>>>> the private data, and the actual configuration will only happen on enable.
>>>>
>>>> We would want set_timings() op to have a direct impact. But we wouldn't
>>>> want the same for setting the data lines, that could be clubbed with
>>>> other configurations at enable. That's okay, right?
>>>
>>> I'm not sure we want/need set_timings to have direct impact. Changing
>>> the timings on the fly has some problems, like the output size changing
>>> to smaller than the overlays, and we perhaps may need to adjust the
>>> clock dividers (dispc's, DSI PLL's or PRCM's).
>>>
>>> It feels just much easier and safer to require that the mgr is disabled
>>> when these changes are made. And as far as I can see, there shouldn't be
>>> any need to change the timings via the shadow registers, as quickly as
>>> possible and during vblank...
>>
>> That makes sense. But currently set_timings for DPI has a direct impact.
>> HDMI/VENC/SDI take the easier route of disabling and enabling the interface.
>>
>> I agree it's safer and easier to make sure things are disabled first,
>> but maybe it's good to have the capability set hdmi timings on the fly
>> in the future, it would make the switch faster, same goes for reading edid.
>
> When do we need to switch mode quickly? Reading edid should not require
> disabling the output for sure.
I think I'm just finding excuses to find a use for my work done in
APPLYing manager related registers.
You are right about edid. Changing HDMI timings take a couple of seconds
now, I was wondering how much that has to do with us completely
disabling/enabling hdmi. it may be just the slowness of the monitors
which causes this.
>
> HDMI is a bit broken currently, though. I think we first enable the
> whole stuff, including video output using VGA, then we read EDID, then
> we change the mode.
>
> We should just enable enough of HDMI to be able to read EDID, and start
> the video output with the correct mode. This needs some restructuring of
> the driver, though. I tried it once quickly, but it turned out not to be
> trivial.
Right. Most likely this restructuring would allow us to modify only the
hdmi timings part when setting a new timing. We could check how much
time we save then :)
>
>> What I meant to ask was whether we should do the same for something like
>> dpi_set_data_lines(), that is, disable dpi, update the data_lines
>> private data with a new value, and enable dpi again.
>
> Hmm, I think it's better to leave disabling and enabling the output to
> the panel driver. So when the panel driver wants to use
> dpi_set_data_lines(), it needs to first disable the DPI output. If it
> doesn't, dpi_set_data_lines() returns -EBUSY.
>
> Otherwise if the panel driver does something like:
>
> dpi_set_foo()
> dpi_set_bar()
>
> Both of those could first disable output, change setting, enable output.
> Instead the panel should first disable, then call those, and then
> enable.
Right, that makes sense.
Archit
^ permalink raw reply
* Re: [rtc-linux] [PATCH 8/8] ARM: vt8500: gpio: Devicetree support for arch-vt8500
From: Arnd Bergmann @ 2012-08-08 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbYxpN9RCu8j8KMBgrb=-DsYZHq7hx-ifmHhJ0ZZcRYjw@mail.gmail.com>
On Wednesday 08 August 2012, Linus Walleij wrote:
> On Wed, Aug 8, 2012 at 3:39 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
>
> > Converted the existing arch-vt8500 gpio to a platform_device.
> > Added support for WM8505 and WM8650 GPIO controllers.
> (...)
> > +++ b/drivers/gpio/gpio-vt8500.c
>
> This driver looks very one-bit-per-gpio typed. Are you sure you cannot
> just reuse drivers/gpio/gpio-generic.c? Make a compelling case please...
>
> > +struct vt8500_gpio_bank_regs {
> > + int en;
> > + int dir;
> > + int data_out;
> > + int data_in;
>
> Why are all these members int? They should be u8 from reading your code.
>
> > + int ngpio;
> > +};
Not necessarily 8 bit, but definitely unsigned.
> > +static struct vt8500_gpio_data vt8500_data = {
> > + .num_banks = 7,
> > + .banks = {
> > + VT8500_BANK(0x00, 0x20, 0x40, 0x60, 26),
> > + VT8500_BANK(0x04, 0x24, 0x44, 0x64, 28),
> > + VT8500_BANK(0x08, 0x28, 0x48, 0x68, 31),
> > + VT8500_BANK(0x0C, 0x2C, 0x4C, 0x6C, 19),
> > + VT8500_BANK(0x10, 0x30, 0x50, 0x70, 19),
> > + VT8500_BANK(0x14, 0x34, 0x54, 0x74, 23),
> > + VT8500_BANK(-1, 0x3C, 0x5C, 0x7C, 9), /* external gpio */
>
> What on earth are all those magic numbers?
>
> I *guess* they're enabling some default GPIO settings etc.
No, they are the register offsets you quoted above, per bank. There
is no easy way to abstract these, and I suggested putting the
values into the source code rather than describing each bank
separately in the .dtsi file.
My feeling however is that the "vt8500_chip->regoff" is wrong, which
would mean only the first bank works. The code adds the same offsets
per bank once more that it sets in this bank table.
Arnd
^ permalink raw reply
* Re: [rtc-linux] [PATCH 8/8] ARM: vt8500: gpio: Devicetree support for arch-vt8500
From: Linus Walleij @ 2012-08-08 9:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1344389967-8465-9-git-send-email-linux@prisktech.co.nz>
On Wed, Aug 8, 2012 at 3:39 AM, Tony Prisk <linux@prisktech.co.nz> wrote:
> Converted the existing arch-vt8500 gpio to a platform_device.
> Added support for WM8505 and WM8650 GPIO controllers.
(...)
> +++ b/drivers/gpio/gpio-vt8500.c
This driver looks very one-bit-per-gpio typed. Are you sure you cannot
just reuse drivers/gpio/gpio-generic.c? Make a compelling case please...
> +struct vt8500_gpio_bank_regs {
> + int en;
> + int dir;
> + int data_out;
> + int data_in;
Why are all these members int? They should be u8 from reading your code.
> + int ngpio;
> +};
> +static struct vt8500_gpio_data vt8500_data = {
> + .num_banks = 7,
> + .banks = {
> + VT8500_BANK(0x00, 0x20, 0x40, 0x60, 26),
> + VT8500_BANK(0x04, 0x24, 0x44, 0x64, 28),
> + VT8500_BANK(0x08, 0x28, 0x48, 0x68, 31),
> + VT8500_BANK(0x0C, 0x2C, 0x4C, 0x6C, 19),
> + VT8500_BANK(0x10, 0x30, 0x50, 0x70, 19),
> + VT8500_BANK(0x14, 0x34, 0x54, 0x74, 23),
> + VT8500_BANK(-1, 0x3C, 0x5C, 0x7C, 9), /* external gpio */
What on earth are all those magic numbers?
I *guess* they're enabling some default GPIO settings etc.
But it really needs better structure, #defines for each one or
atleast include <linux/bitops.h> and say:
= BIT(4) | /* Enable GPIO pin 5 on this bank */
BIT(5); /* Enable GPIO pin 6 on this bank */
However I suspect this is board specific and should
be taken from device tree. Please elaborate on this...
Ditto for the different instances.
(...)
> + unsigned val;
Looks like all of these should be u8.
> + val = readl(vt8500_chip->base + vt8500_chip->regs->en +
> + vt8500_chip->regoff);
val = (u8) readl(...);
usw
> + val |= (1 << offset);
Use <linux/bitops.h>
val |= BIT(offset);
Apart from these remarks it's looking good...
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 8:50 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344413580.4932.5.camel@deskari>
On Wednesday 08 August 2012 01:43 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
>
>> Okay, one thing which I want to align on is that most of these functions
>> don't really do the actual configurations. That is, they'll just update
>> the private data, and the actual configuration will only happen on enable.
>>
>> We would want set_timings() op to have a direct impact. But we wouldn't
>> want the same for setting the data lines, that could be clubbed with
>> other configurations at enable. That's okay, right?
>
> I'm not sure we want/need set_timings to have direct impact. Changing
> the timings on the fly has some problems, like the output size changing
> to smaller than the overlays, and we perhaps may need to adjust the
> clock dividers (dispc's, DSI PLL's or PRCM's).
>
> It feels just much easier and safer to require that the mgr is disabled
> when these changes are made. And as far as I can see, there shouldn't be
> any need to change the timings via the shadow registers, as quickly as
> possible and during vblank...
That makes sense. But currently set_timings for DPI has a direct impact.
HDMI/VENC/SDI take the easier route of disabling and enabling the interface.
I agree it's safer and easier to make sure things are disabled first,
but maybe it's good to have the capability set hdmi timings on the fly
in the future, it would make the switch faster, same goes for reading edid.
What I meant to ask was whether we should do the same for something like
dpi_set_data_lines(), that is, disable dpi, update the data_lines
private data with a new value, and enable dpi again.
>
> This makes me also think that if the output related settings can only be
> changed when the output is off, the apply mechanism is not really needed
> at all for these. Not that it causes any harm, but just a point I
> realized.
Hmm, unfortunately you are right. It's still good to have all the DISPC
writes only in APPLY though, and it gives us the option to do some
operation on the fly if needed in the future.
Archit
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 8:48 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <50222575.6050807@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]
On Wed, 2012-08-08 at 14:08 +0530, Archit Taneja wrote:
> On Wednesday 08 August 2012 01:43 PM, Tomi Valkeinen wrote:
> > On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
> >
> >> Okay, one thing which I want to align on is that most of these functions
> >> don't really do the actual configurations. That is, they'll just update
> >> the private data, and the actual configuration will only happen on enable.
> >>
> >> We would want set_timings() op to have a direct impact. But we wouldn't
> >> want the same for setting the data lines, that could be clubbed with
> >> other configurations at enable. That's okay, right?
> >
> > I'm not sure we want/need set_timings to have direct impact. Changing
> > the timings on the fly has some problems, like the output size changing
> > to smaller than the overlays, and we perhaps may need to adjust the
> > clock dividers (dispc's, DSI PLL's or PRCM's).
> >
> > It feels just much easier and safer to require that the mgr is disabled
> > when these changes are made. And as far as I can see, there shouldn't be
> > any need to change the timings via the shadow registers, as quickly as
> > possible and during vblank...
>
> That makes sense. But currently set_timings for DPI has a direct impact.
> HDMI/VENC/SDI take the easier route of disabling and enabling the interface.
>
> I agree it's safer and easier to make sure things are disabled first,
> but maybe it's good to have the capability set hdmi timings on the fly
> in the future, it would make the switch faster, same goes for reading edid.
When do we need to switch mode quickly? Reading edid should not require
disabling the output for sure.
HDMI is a bit broken currently, though. I think we first enable the
whole stuff, including video output using VGA, then we read EDID, then
we change the mode.
We should just enable enough of HDMI to be able to read EDID, and start
the video output with the correct mode. This needs some restructuring of
the driver, though. I tried it once quickly, but it turned out not to be
trivial.
> What I meant to ask was whether we should do the same for something like
> dpi_set_data_lines(), that is, disable dpi, update the data_lines
> private data with a new value, and enable dpi again.
Hmm, I think it's better to leave disabling and enabling the output to
the panel driver. So when the panel driver wants to use
dpi_set_data_lines(), it needs to first disable the DPI output. If it
doesn't, dpi_set_data_lines() returns -EBUSY.
Otherwise if the panel driver does something like:
dpi_set_foo()
dpi_set_bar()
Both of those could first disable output, change setting, enable output.
Instead the panel should first disable, then call those, and then
enable.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 8:13 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <50221C4D.8000503@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]
On Wed, 2012-08-08 at 13:29 +0530, Archit Taneja wrote:
> Okay, one thing which I want to align on is that most of these functions
> don't really do the actual configurations. That is, they'll just update
> the private data, and the actual configuration will only happen on enable.
>
> We would want set_timings() op to have a direct impact. But we wouldn't
> want the same for setting the data lines, that could be clubbed with
> other configurations at enable. That's okay, right?
I'm not sure we want/need set_timings to have direct impact. Changing
the timings on the fly has some problems, like the output size changing
to smaller than the overlays, and we perhaps may need to adjust the
clock dividers (dispc's, DSI PLL's or PRCM's).
It feels just much easier and safer to require that the mgr is disabled
when these changes are made. And as far as I can see, there shouldn't be
any need to change the timings via the shadow registers, as quickly as
possible and during vblank...
This makes me also think that if the output related settings can only be
changed when the output is off, the apply mechanism is not really needed
at all for these. Not that it causes any harm, but just a point I
realized.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 8:11 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344410835.17575.54.camel@lappyti>
On Wednesday 08 August 2012 12:57 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 12:17 +0530, Archit Taneja wrote:
>> On Wednesday 08 August 2012 11:55 AM, Tomi Valkeinen wrote:
>>> On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
>>>> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
>>>>> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>>>>>> This series tries to make interface drivers less dependent on omap_dss_device
>>>>>> which represents a panel/device connected to that interface. The current way of
>>>>>> configuring an interface is to populate the panel's omap_dss_device instance
>>>>>> with parameters common to the panel and the interface, they are either populated
>>>>>> in the board file, or in the panel driver. Panel timings, number of lanes
>>>>>> connected to interface, and pixel format are examples of such parameters, these
>>>>>> are then extracted by the interface driver to configure itself.
>>>>>
>>>>> The series looks good. I had only a few comments to make, but obviously
>>>>> this needs quite a bit of testing. I'll try it out.
>>>>
>>>> One thing I'm not sure about is whether these new functions should be
>>>> aware of the state of the output. For example, if we call set_timings()
>>>> with DSI video mode which is already enabled, the timings won't really
>>>> take any impact.
>>>>
>>>> Similar issues would occur when we try to make other ops like
>>>> set_data_lines() or set_pixel_format(). These need to be called before
>>>> the output is enabled. I was wondering if we would need to add
>>>> intelligence here to make panel drivers less likely to make mistakes.
>>>
>>> Hmm, true. It'd be nice if the functions returned -EBUSY if the
>>> operation cannot be done while the output is enabled.
>>>
>>> We have the dssdev->state, but we should get rid of that (or leave it to
>>> panel drivers). It'd be good if the output drivers know whether the
>>> output is enabled or not. I think this data is already tracked by
>>> apply.c. It's about ovl managers, but I think that's practically the
>>> same as output.
>>>
>>> Calling dss_mgr_enable() will set mp->enabled = true, which could be
>>> returned via dss_mgr_is_enabled() or such.
>>>
>>> Then again, it wouldn't be many lines of codes to track the enable-state
>>> in each output driver. So if we have any suspicions that mp->enabled
>>> doesn't quite work for, say, dsi, we could just add a private "enabled"
>>> member to dsi. But I don't right away see why dss_mgr_is_enabled()
>>> wouldn't work.
>>>
>>
>> I think we had discussed previously that it may not the best idea to see
>> if a manager is enabled via mp->enabled as it's always possible that it
>> changes afterwards. Same for any other parameter in APPLY's private
>> data. This was the reason why we passed privtate data to DISPC functions
>> rather than creating apply helper functions which return the value of a
>> private data. For example, we pass manager timings to dispc_ovl_setup(),
>> instead of DISPC using a function like dss_mgr_get_timings().
>
> I think that's slightly different problem. The dispc case has an issue
> with locking. If dispc_ovl_setup() is called with the apply's spinlock
> taken, neither dispc_ovl_setup() nor dss_mgr_get_timings() can take the
> lock. But if dispc_ovl_setup() is called from somewhere else, it should
> take the lock. Also, if dispc_ovl_setup() would call a function in
> apply, it'd be calling "upwards" to a higher level component.
>
> With the output driver calling apply, none of those problems is present,
> I believe.
>
>> I also don't see why dss_mgr_is_enabled() wouldn't work. The only places
>> where the manager's state will change are the output's enable and
>> disable ops. The mutex maintained by the output would ensure
>> sequential-ity between the output's enable() and set_timings() op, and
>> hence ensure the manager's state we see is fine.
>>
>> If we manage the 'enabled' state for each output interface, we would be
>> a bit more consistent with respect to other parameters. For example,
>> timings is maintained by both manager and the output. Also, if we need
>> to separate out manager configurations from outputs in the future, it
>> would probably be better for the output to query it's own state rather
>> than depending on the manager, which could be configured either earlier
>> or later.
>
> Two things that came to my mind:
>
> If the output driver uses dss_mgr_is_enabled(), if both DPI and DSI
> output drivers use the same manager, they'd both see themselves as
> enabled. Of course only one can work at a time, so I'm not sure if
> that's a practical problem. And if we had some kind of link between the
> mgr and the output driver this would not be an issue.
>
> The second thing is that we're not strictly required to have DISPC
> connected to DSI or RFBI. We could use CPU/sDMA to output the image.
> This is quite theoretical, though.
>
> So, I think using dss_mgr_is_enabled() would work, but I'm still not
> 100% sure...
>
> Well, perhaps the code should be such that dss_mgr_is_enabled() is used
> to see if the mgr is enabled, not if the output is enabled. What I mean
> with this is that if, say, set_data_lines() calls dispc to set the data
> lines, we are really interested in if the dispc's mgr is enabled, not if
> the DSI is enabled.
>
> And if some other function changes DSI configuration (but doesn't touch
> dispc), then we're not really interested in if the mgr is enabled, but
> if the DSI is enabled.
>
> That's a bit more complex than using only dss_mgr_is_enabled() or using
> only output specific enable-flag, but I think it's more correct. In
> DPI's case only dss_mgr_is_enabled() is probably needed. For DSI we may
> need a separate private enable-flag.
Okay, one thing which I want to align on is that most of these functions
don't really do the actual configurations. That is, they'll just update
the private data, and the actual configuration will only happen on enable.
We would want set_timings() op to have a direct impact. But we wouldn't
want the same for setting the data lines, that could be clubbed with
other configurations at enable. That's okay, right?
Archit
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Archit Taneja @ 2012-08-08 8:06 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344409847.17575.40.camel@lappyti>
On Wednesday 08 August 2012 12:40 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 11:59 +0530, Archit Taneja wrote:
>
>> I tried out rotation on Taal, and it only works for 180 degrees(and 0 of
>> course), 90 and 270 result in no output. I'll add a
>> dss_mgr_set_timings() in omap_dsi_update, that should sort of fix it,
>> but someone would need to reconfigure the connected overlays too before
>> trying out an update.
>
> Right, but that's something omapdss/panel cannot do, it must be done by
> the user. The same problem is there with changing, say, DPI mode also.
>
> Btw, can you separate smaller cleanups/fixes to another patch series, to
> make this series even slightly smaller? I think at least the first
> patches in this series are quite separate, and the rotation fix is also.
Right, I'll do that.
Archit
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 7:27 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <50220B94.5040303@ti.com>
[-- Attachment #1: Type: text/plain, Size: 5740 bytes --]
On Wed, 2012-08-08 at 12:17 +0530, Archit Taneja wrote:
> On Wednesday 08 August 2012 11:55 AM, Tomi Valkeinen wrote:
> > On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
> >> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
> >>> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
> >>>> This series tries to make interface drivers less dependent on omap_dss_device
> >>>> which represents a panel/device connected to that interface. The current way of
> >>>> configuring an interface is to populate the panel's omap_dss_device instance
> >>>> with parameters common to the panel and the interface, they are either populated
> >>>> in the board file, or in the panel driver. Panel timings, number of lanes
> >>>> connected to interface, and pixel format are examples of such parameters, these
> >>>> are then extracted by the interface driver to configure itself.
> >>>
> >>> The series looks good. I had only a few comments to make, but obviously
> >>> this needs quite a bit of testing. I'll try it out.
> >>
> >> One thing I'm not sure about is whether these new functions should be
> >> aware of the state of the output. For example, if we call set_timings()
> >> with DSI video mode which is already enabled, the timings won't really
> >> take any impact.
> >>
> >> Similar issues would occur when we try to make other ops like
> >> set_data_lines() or set_pixel_format(). These need to be called before
> >> the output is enabled. I was wondering if we would need to add
> >> intelligence here to make panel drivers less likely to make mistakes.
> >
> > Hmm, true. It'd be nice if the functions returned -EBUSY if the
> > operation cannot be done while the output is enabled.
> >
> > We have the dssdev->state, but we should get rid of that (or leave it to
> > panel drivers). It'd be good if the output drivers know whether the
> > output is enabled or not. I think this data is already tracked by
> > apply.c. It's about ovl managers, but I think that's practically the
> > same as output.
> >
> > Calling dss_mgr_enable() will set mp->enabled = true, which could be
> > returned via dss_mgr_is_enabled() or such.
> >
> > Then again, it wouldn't be many lines of codes to track the enable-state
> > in each output driver. So if we have any suspicions that mp->enabled
> > doesn't quite work for, say, dsi, we could just add a private "enabled"
> > member to dsi. But I don't right away see why dss_mgr_is_enabled()
> > wouldn't work.
> >
>
> I think we had discussed previously that it may not the best idea to see
> if a manager is enabled via mp->enabled as it's always possible that it
> changes afterwards. Same for any other parameter in APPLY's private
> data. This was the reason why we passed privtate data to DISPC functions
> rather than creating apply helper functions which return the value of a
> private data. For example, we pass manager timings to dispc_ovl_setup(),
> instead of DISPC using a function like dss_mgr_get_timings().
I think that's slightly different problem. The dispc case has an issue
with locking. If dispc_ovl_setup() is called with the apply's spinlock
taken, neither dispc_ovl_setup() nor dss_mgr_get_timings() can take the
lock. But if dispc_ovl_setup() is called from somewhere else, it should
take the lock. Also, if dispc_ovl_setup() would call a function in
apply, it'd be calling "upwards" to a higher level component.
With the output driver calling apply, none of those problems is present,
I believe.
> I also don't see why dss_mgr_is_enabled() wouldn't work. The only places
> where the manager's state will change are the output's enable and
> disable ops. The mutex maintained by the output would ensure
> sequential-ity between the output's enable() and set_timings() op, and
> hence ensure the manager's state we see is fine.
>
> If we manage the 'enabled' state for each output interface, we would be
> a bit more consistent with respect to other parameters. For example,
> timings is maintained by both manager and the output. Also, if we need
> to separate out manager configurations from outputs in the future, it
> would probably be better for the output to query it's own state rather
> than depending on the manager, which could be configured either earlier
> or later.
Two things that came to my mind:
If the output driver uses dss_mgr_is_enabled(), if both DPI and DSI
output drivers use the same manager, they'd both see themselves as
enabled. Of course only one can work at a time, so I'm not sure if
that's a practical problem. And if we had some kind of link between the
mgr and the output driver this would not be an issue.
The second thing is that we're not strictly required to have DISPC
connected to DSI or RFBI. We could use CPU/sDMA to output the image.
This is quite theoretical, though.
So, I think using dss_mgr_is_enabled() would work, but I'm still not
100% sure...
Well, perhaps the code should be such that dss_mgr_is_enabled() is used
to see if the mgr is enabled, not if the output is enabled. What I mean
with this is that if, say, set_data_lines() calls dispc to set the data
lines, we are really interested in if the dispc's mgr is enabled, not if
the DSI is enabled.
And if some other function changes DSI configuration (but doesn't touch
dispc), then we're not really interested in if the mgr is enabled, but
if the DSI is enabled.
That's a bit more complex than using only dss_mgr_is_enabled() or using
only output specific enable-flag, but I think it's more correct. In
DPI's case only dss_mgr_is_enabled() is probably needed. For DSI we may
need a separate private enable-flag.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Tomi Valkeinen @ 2012-08-08 7:10 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <5022073B.6070109@ti.com>
[-- Attachment #1: Type: text/plain, Size: 743 bytes --]
On Wed, 2012-08-08 at 11:59 +0530, Archit Taneja wrote:
> I tried out rotation on Taal, and it only works for 180 degrees(and 0 of
> course), 90 and 270 result in no output. I'll add a
> dss_mgr_set_timings() in omap_dsi_update, that should sort of fix it,
> but someone would need to reconfigure the connected overlays too before
> trying out an update.
Right, but that's something omapdss/panel cannot do, it must be done by
the user. The same problem is there with changing, say, DPI mode also.
Btw, can you separate smaller cleanups/fixes to another patch series, to
make this series even slightly smaller? I think at least the first
patches in this series are quite separate, and the rotation fix is also.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 6:59 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344407158.17575.21.camel@lappyti>
On Wednesday 08 August 2012 11:55 AM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
>> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
>>> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>>>> This series tries to make interface drivers less dependent on omap_dss_device
>>>> which represents a panel/device connected to that interface. The current way of
>>>> configuring an interface is to populate the panel's omap_dss_device instance
>>>> with parameters common to the panel and the interface, they are either populated
>>>> in the board file, or in the panel driver. Panel timings, number of lanes
>>>> connected to interface, and pixel format are examples of such parameters, these
>>>> are then extracted by the interface driver to configure itself.
>>>
>>> The series looks good. I had only a few comments to make, but obviously
>>> this needs quite a bit of testing. I'll try it out.
>>
>> One thing I'm not sure about is whether these new functions should be
>> aware of the state of the output. For example, if we call set_timings()
>> with DSI video mode which is already enabled, the timings won't really
>> take any impact.
>>
>> Similar issues would occur when we try to make other ops like
>> set_data_lines() or set_pixel_format(). These need to be called before
>> the output is enabled. I was wondering if we would need to add
>> intelligence here to make panel drivers less likely to make mistakes.
>
> Hmm, true. It'd be nice if the functions returned -EBUSY if the
> operation cannot be done while the output is enabled.
>
> We have the dssdev->state, but we should get rid of that (or leave it to
> panel drivers). It'd be good if the output drivers know whether the
> output is enabled or not. I think this data is already tracked by
> apply.c. It's about ovl managers, but I think that's practically the
> same as output.
>
> Calling dss_mgr_enable() will set mp->enabled = true, which could be
> returned via dss_mgr_is_enabled() or such.
>
> Then again, it wouldn't be many lines of codes to track the enable-state
> in each output driver. So if we have any suspicions that mp->enabled
> doesn't quite work for, say, dsi, we could just add a private "enabled"
> member to dsi. But I don't right away see why dss_mgr_is_enabled()
> wouldn't work.
>
I think we had discussed previously that it may not the best idea to see
if a manager is enabled via mp->enabled as it's always possible that it
changes afterwards. Same for any other parameter in APPLY's private
data. This was the reason why we passed privtate data to DISPC functions
rather than creating apply helper functions which return the value of a
private data. For example, we pass manager timings to dispc_ovl_setup(),
instead of DISPC using a function like dss_mgr_get_timings().
I also don't see why dss_mgr_is_enabled() wouldn't work. The only places
where the manager's state will change are the output's enable and
disable ops. The mutex maintained by the output would ensure
sequential-ity between the output's enable() and set_timings() op, and
hence ensure the manager's state we see is fine.
If we manage the 'enabled' state for each output interface, we would be
a bit more consistent with respect to other parameters. For example,
timings is maintained by both manager and the output. Also, if we need
to separate out manager configurations from outputs in the future, it
would probably be better for the output to query it's own state rather
than depending on the manager, which could be configured either earlier
or later.
Archit
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Archit Taneja @ 2012-08-08 6:41 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344406508.17575.12.camel@lappyti>
On Wednesday 08 August 2012 11:45 AM, Tomi Valkeinen wrote:
> On Wed, 2012-08-08 at 11:27 +0530, Archit Taneja wrote:
>
>> I am a bit unclear about resolution when it comes to command mode panels.
>
> Right, it's a bit confusing. And I'm not 100% sure how to manage the
> rotation.
>
>> For command mode panels, we can perform rotation at the panel side. That
>> is, the panel refreshes itself by fetching pixels from it's buffer in a
>> rotated way. Is that right?
>
> Yes. Well, actually I think the panel stores the pixels in rotated
> manner when it receives them from OMAP, but it's practically the same.
>
> One thing to realize is that this kind of rotation is a bit limited:
> because there's only one buffer, OMAP will write pixels to the buffers
> at the same time as the panel shows them. When rotating, this leads to
> tearing.
>
> If the panel has double buffer, that solves the problem, but I haven't
> seen such panels. Another option is to update the panel in two parts,
> like N9 does, but that's timing sensitive and a bit tricky.
>
>> If the original resolution is 864x480, and we set rotation at panel side
>> to make the rotation 480x864, the DISPC manager size should also be
>> configured at 480x864 right?
>
> Yep. When we use the panel rotation, from OMAP's point of view the panel
> resolution has changed.
>
>> We seem to be setting the manager timings only once when DSI is enabled.
>> After that, setting rotation doesn't impact manager size.
>
> Hmm, previously the mgr size was set before each update. I wonder if
> that code has been dropped, probably because we removed the support for
> partial updates at one point. Without partial updates, the size stays
> the same, except obviously with rotation. I think I just forgot about
> rotation at that time.
I tried out rotation on Taal, and it only works for 180 degrees(and 0 of
course), 90 and 270 result in no output. I'll add a
dss_mgr_set_timings() in omap_dsi_update, that should sort of fix it,
but someone would need to reconfigure the connected overlays too before
trying out an update.
>
>> I am asking this to understand if we need to keep resolution as a
>> separate parameter than timings. That is, timings represents the initial
>> width and height of the panel, and resolution represents the current
>> width and height of the panel.
>
> I'm not sure. I think that OMAP doesn't really need to know about the
> initial resolution. It doesn't really matter from OMAP's point of view.
>
> I think I originally kept timings and resolution separately, and the
> idea was that timings represent the panel's timings, i.e. how it updates
> the screen from its own memory. And resolution represents the usable
> resolution, from OMAP's point of view.
>
> While I haven't seen such a cmd mode panel, there could be a command
> sent to the panel to configure its timings. For this we need real
> timings, not the rotated resolution.
>
> However, even in that case the DISPC doesn't need to know about those
> timings, they would be handled by the panel driver (which could,
> perhaps, reconfigure the DSI bus speed to match the new timings). So I
> think that inside omapdss, we don't need separate timings and resolution
> for DSI cmd mode panels.
Ok.
Archit
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Tomi Valkeinen @ 2012-08-08 6:25 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <502201B7.9030205@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]
On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
> > On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
> >> This series tries to make interface drivers less dependent on omap_dss_device
> >> which represents a panel/device connected to that interface. The current way of
> >> configuring an interface is to populate the panel's omap_dss_device instance
> >> with parameters common to the panel and the interface, they are either populated
> >> in the board file, or in the panel driver. Panel timings, number of lanes
> >> connected to interface, and pixel format are examples of such parameters, these
> >> are then extracted by the interface driver to configure itself.
> >
> > The series looks good. I had only a few comments to make, but obviously
> > this needs quite a bit of testing. I'll try it out.
>
> One thing I'm not sure about is whether these new functions should be
> aware of the state of the output. For example, if we call set_timings()
> with DSI video mode which is already enabled, the timings won't really
> take any impact.
>
> Similar issues would occur when we try to make other ops like
> set_data_lines() or set_pixel_format(). These need to be called before
> the output is enabled. I was wondering if we would need to add
> intelligence here to make panel drivers less likely to make mistakes.
Hmm, true. It'd be nice if the functions returned -EBUSY if the
operation cannot be done while the output is enabled.
We have the dssdev->state, but we should get rid of that (or leave it to
panel drivers). It'd be good if the output drivers know whether the
output is enabled or not. I think this data is already tracked by
apply.c. It's about ovl managers, but I think that's practically the
same as output.
Calling dss_mgr_enable() will set mp->enabled = true, which could be
returned via dss_mgr_is_enabled() or such.
Then again, it wouldn't be many lines of codes to track the enable-state
in each output driver. So if we have any suspicions that mp->enabled
doesn't quite work for, say, dsi, we could just add a private "enabled"
member to dsi. But I don't right away see why dss_mgr_is_enabled()
wouldn't work.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [PATCH V4 0/2] arm: samsung: Move FIMD headers to include/video/
From: Kukjin Kim @ 2012-08-08 6:20 UTC (permalink / raw)
To: 'Leela Krishna Amudala', linux-arm-kernel,
linux-samsung-soc
Cc: dri-devel, linux-fbdev, ben-linux, m.szyprowski,
'Florian Tobias Schandinat'
In-Reply-To: <1344342872-7904-1-git-send-email-l.krishna@samsung.com>
Leela Krishna Amudala wrote:
>
> This patchset moves the contents of regs-fb-v4.h and regs-fb.h from arch
> side
> to include/video/samsung_fimd.h
>
> This patchset is created and rebased against master branch of torvalds
> tree.
> Tested on smdk5250 board, build tested for other boards.
>
> Changes from version 3:
> - Squashed 1st, 3rd, 4th and 5th patches from version 3
>
> Changes from version 2:
> - Changed the EXYNOS5_xxx macro name to FIMD_V8_xxx
> - Moved the header inclusion under same kind
>
> Changes from version 1:
> - Split the patches as per Sylwester comments
> - Changed FIMD_V8_xxx macro to EXYNOS5_xxx
>
> Leela Krishna Amudala (2):
> include/video: move fimd register headers from platform to
> include/video
> include/video: Add register offsets for FIMD version 8
>
> arch/arm/mach-exynos/mach-nuri.c | 2 +-
> arch/arm/mach-exynos/mach-origen.c | 2 +-
> arch/arm/mach-exynos/mach-smdk4x12.c | 2 +-
> arch/arm/mach-exynos/mach-smdkv310.c | 2 +-
> arch/arm/mach-exynos/mach-universal_c210.c | 2 +-
> arch/arm/mach-exynos/setup-fimd0.c | 2 +-
> arch/arm/mach-s3c24xx/mach-smdk2416.c | 2 +-
> arch/arm/mach-s3c64xx/mach-anw6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-crag6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-hmt.c | 2 +-
> arch/arm/mach-s3c64xx/mach-mini6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-ncp.c | 2 +-
> arch/arm/mach-s3c64xx/mach-real6410.c | 2 +-
> arch/arm/mach-s3c64xx/mach-smartq5.c | 2 +-
> arch/arm/mach-s3c64xx/mach-smartq7.c | 2 +-
> arch/arm/mach-s3c64xx/mach-smdk6410.c | 2 +-
> arch/arm/mach-s5p64x0/mach-smdk6440.c | 2 +-
> arch/arm/mach-s5p64x0/mach-smdk6450.c | 2 +-
> arch/arm/mach-s5pc100/mach-smdkc100.c | 2 +-
> arch/arm/mach-s5pv210/mach-aquila.c | 2 +-
> arch/arm/mach-s5pv210/mach-goni.c | 2 +-
> arch/arm/mach-s5pv210/mach-smdkv210.c | 2 +-
> arch/arm/plat-samsung/include/plat/regs-fb-v4.h | 159
----------------
> ----
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 +-
> drivers/video/s3c-fb.c | 2 +-
> .../plat/regs-fb.h => include/video/samsung_fimd.h | 152
> +++++++++++++++++--
> 26 files changed, 165 insertions(+), 194 deletions(-)
> delete mode 100644 arch/arm/plat-samsung/include/plat/regs-fb-v4.h
> rename arch/arm/plat-samsung/include/plat/regs-fb.h =>
> include/video/samsung_fimd.h (73%)
Looks OK to me. Will apply into topic branch, 'v3.7-for-florian' for
Florian's pulling.
If there is no objection for a couple of days, I will merge this into
for-next and send to Florian.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply
* Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface
From: Archit Taneja @ 2012-08-08 6:17 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344349948.7216.82.camel@lappyti>
On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>> This series tries to make interface drivers less dependent on omap_dss_device
>> which represents a panel/device connected to that interface. The current way of
>> configuring an interface is to populate the panel's omap_dss_device instance
>> with parameters common to the panel and the interface, they are either populated
>> in the board file, or in the panel driver. Panel timings, number of lanes
>> connected to interface, and pixel format are examples of such parameters, these
>> are then extracted by the interface driver to configure itself.
>
> The series looks good. I had only a few comments to make, but obviously
> this needs quite a bit of testing. I'll try it out.
One thing I'm not sure about is whether these new functions should be
aware of the state of the output. For example, if we call set_timings()
with DSI video mode which is already enabled, the timings won't really
take any impact.
Similar issues would occur when we try to make other ops like
set_data_lines() or set_pixel_format(). These need to be called before
the output is enabled. I was wondering if we would need to add
intelligence here to make panel drivers less likely to make mistakes.
Archit
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Tomi Valkeinen @ 2012-08-08 6:15 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <5021FFDC.6080006@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2871 bytes --]
On Wed, 2012-08-08 at 11:27 +0530, Archit Taneja wrote:
> I am a bit unclear about resolution when it comes to command mode panels.
Right, it's a bit confusing. And I'm not 100% sure how to manage the
rotation.
> For command mode panels, we can perform rotation at the panel side. That
> is, the panel refreshes itself by fetching pixels from it's buffer in a
> rotated way. Is that right?
Yes. Well, actually I think the panel stores the pixels in rotated
manner when it receives them from OMAP, but it's practically the same.
One thing to realize is that this kind of rotation is a bit limited:
because there's only one buffer, OMAP will write pixels to the buffers
at the same time as the panel shows them. When rotating, this leads to
tearing.
If the panel has double buffer, that solves the problem, but I haven't
seen such panels. Another option is to update the panel in two parts,
like N9 does, but that's timing sensitive and a bit tricky.
> If the original resolution is 864x480, and we set rotation at panel side
> to make the rotation 480x864, the DISPC manager size should also be
> configured at 480x864 right?
Yep. When we use the panel rotation, from OMAP's point of view the panel
resolution has changed.
> We seem to be setting the manager timings only once when DSI is enabled.
> After that, setting rotation doesn't impact manager size.
Hmm, previously the mgr size was set before each update. I wonder if
that code has been dropped, probably because we removed the support for
partial updates at one point. Without partial updates, the size stays
the same, except obviously with rotation. I think I just forgot about
rotation at that time.
> I am asking this to understand if we need to keep resolution as a
> separate parameter than timings. That is, timings represents the initial
> width and height of the panel, and resolution represents the current
> width and height of the panel.
I'm not sure. I think that OMAP doesn't really need to know about the
initial resolution. It doesn't really matter from OMAP's point of view.
I think I originally kept timings and resolution separately, and the
idea was that timings represent the panel's timings, i.e. how it updates
the screen from its own memory. And resolution represents the usable
resolution, from OMAP's point of view.
While I haven't seen such a cmd mode panel, there could be a command
sent to the panel to configure its timings. For this we need real
timings, not the rotated resolution.
However, even in that case the DISPC doesn't need to know about those
timings, they would be handled by the panel driver (which could,
perhaps, reconfigure the DSI bus speed to match the new timings). So I
think that inside omapdss, we don't need separate timings and resolution
for DSI cmd mode panels.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 13/17] OMAPDSS: SDI: Create a function to set timings
From: Archit Taneja @ 2012-08-08 6:10 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344349218.7216.80.camel@lappyti>
On Tuesday 07 August 2012 07:50 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>> Create function omapdss_sdi_set_timings(), this can be used by a SDI panel
>> driver without disabling/enabling the SDI interface. This is similar to the
>> set_timings op of the DPI interface driver. It calls sdi_set_mode() which only
>> configures the DISPC timings and DSS/DISPC clock dividers.
>
> I don't think this works, as the SDI PLL uses pclk-free, and if pclk
> changes, PLL lock probably breaks.
>
> OMAP3430 TRM explains the sequence how to configure settings on the fly,
> but that's not very simple. Just turning the output off and on is much
> easier.
Right, I'll make set_timings() just disable and enable SDI like before.
Archit
^ permalink raw reply
* Re: [RFC 08/17] OMAPDSS: DSI: Maintain own copy of timings in driver data
From: Archit Taneja @ 2012-08-08 6:09 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, sumit.semwal, rob
In-Reply-To: <1344348462.7216.77.camel@lappyti>
On Tuesday 07 August 2012 07:37 PM, Tomi Valkeinen wrote:
> On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
>> The DSI driver currently relies on the timings in omap_dss_device struct to
>> configure the DISPC and DSI blocks accordingly. This makes the DSI interface
>> driver dependent on the omap_dss_device struct.
>>
>> Make the DPI driver data maintain it's own timings field. The panel driver is
> ^^^
> DSI
>
>> expected to call omapdss_dsi_set_timings() to set these timings before the panel
>> is enabled.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>d
>> ---
>> drivers/video/omap2/displays/panel-taal.c | 2 ++
>> drivers/video/omap2/dss/dsi.c | 27 ++++++++++++++++++++++-----
>> include/video/omapdss.h | 2 ++
>> 3 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
>> index 3f5acc7..4775580 100644
>> --- a/drivers/video/omap2/displays/panel-taal.c
>> +++ b/drivers/video/omap2/displays/panel-taal.c
>> @@ -1060,6 +1060,8 @@ static int taal_power_on(struct omap_dss_device *dssdev)
>> goto err0;
>> };
>>
>> + omapdss_dsi_set_timings(dssdev, &td->panel_config->timings);
>> +
>> r = omapdss_dsi_display_enable(dssdev);
>> if (r) {
>> dev_err(&dssdev->dev, "failed to enable DSI\n");
>
> Video timings for command mode panel are meaningless. If we need to pass
> the resolution of the panel, perhaps we should have a separate function
> for that.
>
> However, with a quick glance at dsi.c, we don't even use the
> dssdev->panel.timings for cmd mode panel. But we do use
> dssdev->get_resolution() in a few places. Those calls could be replaced
> by storing the panel size in dsi.c, given with omapdss_dsi_set_size() or
> such. We could use the timings field in dsi.c to store them, though.
I am a bit unclear about resolution when it comes to command mode panels.
For command mode panels, we can perform rotation at the panel side. That
is, the panel refreshes itself by fetching pixels from it's buffer in a
rotated way. Is that right?
If the original resolution is 864x480, and we set rotation at panel side
to make the rotation 480x864, the DISPC manager size should also be
configured at 480x864 right?
We seem to be setting the manager timings only once when DSI is enabled.
After that, setting rotation doesn't impact manager size.
I am asking this to understand if we need to keep resolution as a
separate parameter than timings. That is, timings represents the initial
width and height of the panel, and resolution represents the current
width and height of the panel.
Archit
^ permalink raw reply
* [PATCH 10/10] video: exynos_dp: Enable hotplug interrupts
From: Sean Paul @ 2012-08-08 3:54 UTC (permalink / raw)
To: linux-fbdev
Enable hotplug interrupts and move the hotplug scheduling into the
interrupt handler. This allows us to introduce a screen at any time
while we're running.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 40 +++++++++++++++++++++++++-------
drivers/video/exynos/exynos_dp_core.h | 9 +++++++
drivers/video/exynos/exynos_dp_reg.c | 35 ++++++++++++++++++++++++++++-
3 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index eb5816f..5279027 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -47,10 +47,6 @@ static int exynos_dp_detect_hpd(struct exynos_dp_device *dp)
{
int timeout_loop = 0;
- exynos_dp_init_hpd(dp);
-
- usleep_range(200, 210);
-
while (exynos_dp_get_plug_in_status(dp) != 0) {
timeout_loop++;
if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
@@ -829,8 +825,32 @@ static void exynos_dp_enable_scramble(struct exynos_dp_device *dp, bool enable)
static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
{
struct exynos_dp_device *dp = arg;
-
- dev_err(dp->dev, "exynos_dp_irq_handler\n");
+ enum dp_irq_type irq_type;
+
+ irq_type = exynos_dp_get_irq_type(dp);
+ switch (irq_type) {
+ case DP_IRQ_TYPE_HP_CABLE_IN:
+ dev_dbg(dp->dev, "Received irq - cable in\n");
+ schedule_work(&dp->hotplug_work);
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ case DP_IRQ_TYPE_HP_CABLE_OUT:
+ dev_dbg(dp->dev, "Received irq - cable out\n");
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ case DP_IRQ_TYPE_HP_CHANGE:
+ /*
+ * We get these change notifications once in a while, but there
+ * is nothing we can do with them. Just ignore it for now and
+ * only handle cable changes.
+ */
+ dev_dbg(dp->dev, "Received irq - hotplug change; ignoring.\n");
+ exynos_dp_clear_hotplug_interrupts(dp);
+ break;
+ default:
+ dev_err(dp->dev, "Received irq - unknown type!\n");
+ break;
+ }
return IRQ_HANDLED;
}
@@ -843,7 +863,7 @@ static void exynos_dp_hotplug(struct work_struct *work)
ret = exynos_dp_detect_hpd(dp);
if (ret) {
- dev_err(dp->dev, "unable to detect hpd\n");
+ /* Cable has been disconnected, we're done */
return;
}
@@ -941,7 +961,6 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
exynos_dp_init_dp(dp);
platform_set_drvdata(pdev, dp);
- schedule_work(&dp->hotplug_work);
return 0;
@@ -975,6 +994,8 @@ static int exynos_dp_suspend(struct device *dev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ disable_irq(dp->irq);
+
if (work_pending(&dp->hotplug_work))
flush_work_sync(&dp->hotplug_work);
@@ -998,7 +1019,8 @@ static int exynos_dp_resume(struct device *dev)
clk_enable(dp->clock);
exynos_dp_init_dp(dp);
- schedule_work(&dp->hotplug_work);
+
+ enable_irq(dp->irq);
return 0;
}
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index cf1010b..a5795dd 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -13,6 +13,13 @@
#ifndef _EXYNOS_DP_CORE_H
#define _EXYNOS_DP_CORE_H
+enum dp_irq_type {
+ DP_IRQ_TYPE_HP_CABLE_IN,
+ DP_IRQ_TYPE_HP_CABLE_OUT,
+ DP_IRQ_TYPE_HP_CHANGE,
+ DP_IRQ_TYPE_UNKNOWN,
+};
+
struct link_train {
int eq_loop;
int cr_loop[4];
@@ -51,6 +58,8 @@ void exynos_dp_set_analog_power_down(struct exynos_dp_device *dp,
bool enable);
void exynos_dp_init_analog_func(struct exynos_dp_device *dp);
void exynos_dp_init_hpd(struct exynos_dp_device *dp);
+enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp);
+void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp);
void exynos_dp_reset_aux(struct exynos_dp_device *dp);
void exynos_dp_init_aux(struct exynos_dp_device *dp);
int exynos_dp_get_plug_in_status(struct exynos_dp_device *dp);
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 389e0f0..5906fc7 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -327,7 +327,7 @@ void exynos_dp_init_analog_func(struct exynos_dp_device *dp)
writel(reg, dp->reg_base + EXYNOS_DP_FUNC_EN_2);
}
-void exynos_dp_init_hpd(struct exynos_dp_device *dp)
+void exynos_dp_clear_hotplug_interrupts(struct exynos_dp_device *dp)
{
u32 reg;
@@ -336,10 +336,43 @@ void exynos_dp_init_hpd(struct exynos_dp_device *dp)
reg = INT_HPD;
writel(reg, dp->reg_base + EXYNOS_DP_INT_STA);
+}
+
+void exynos_dp_init_hpd(struct exynos_dp_device *dp)
+{
+ u32 reg;
+
+ exynos_dp_clear_hotplug_interrupts(dp);
reg = readl(dp->reg_base + EXYNOS_DP_SYS_CTL_3);
reg &= ~(F_HPD | HPD_CTRL);
writel(reg, dp->reg_base + EXYNOS_DP_SYS_CTL_3);
+
+ /* Unmask hotplug interrupts */
+ reg = HOTPLUG_CHG | HPD_LOST | PLUG;
+ writel(reg, dp->reg_base + EXYNOS_DP_COMMON_INT_MASK_4);
+
+ reg = INT_HPD;
+ writel(reg, dp->reg_base + EXYNOS_DP_INT_STA_MASK);
+}
+
+enum dp_irq_type exynos_dp_get_irq_type(struct exynos_dp_device *dp)
+{
+ u32 reg;
+
+ /* Parse hotplug interrupt status register */
+ reg = readl(dp->reg_base + EXYNOS_DP_COMMON_INT_STA_4);
+
+ if (reg & PLUG)
+ return DP_IRQ_TYPE_HP_CABLE_IN;
+
+ if (reg & HPD_LOST)
+ return DP_IRQ_TYPE_HP_CABLE_OUT;
+
+ if (reg & HOTPLUG_CHG)
+ return DP_IRQ_TYPE_HP_CHANGE;
+
+ return DP_IRQ_TYPE_UNKNOWN;
}
void exynos_dp_reset_aux(struct exynos_dp_device *dp)
--
1.7.7.3
^ permalink raw reply related
* [PATCH 09/10] video: exynos_dp: Fix bug when checking dp->irq
From: Sean Paul @ 2012-08-08 3:54 UTC (permalink / raw)
To: linux-fbdev
Fix a bug where we check !dp->irq instead of the correct check for
-ENXIO.
BUG=None
TEST=None
Change-Id: Ibcdd16fb71eb41e4380ac8911ad6063634ff80e2
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/29144
Reviewed-by: Olof Johansson <olofj@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 68ad494..eb5816f 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -919,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
}
dp->irq = platform_get_irq(pdev, 0);
- if (!dp->irq) {
+ if (dp->irq = -ENXIO) {
dev_err(&pdev->dev, "failed to get irq\n");
ret = -ENODEV;
goto err_clock;
--
1.7.7.3
^ permalink raw reply related
* [PATCH 08/10] video: exynos_dp: Move hotplug into a workqueue
From: Sean Paul @ 2012-08-08 3:54 UTC (permalink / raw)
To: linux-fbdev
Move the hotplug related code from probe and resume into a workqueue.
This allows us to initialize the DP driver (and resume it) when there
is no monitor connected.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 94 +++++++++++++++++----------------
drivers/video/exynos/exynos_dp_core.h | 1 +
2 files changed, 50 insertions(+), 45 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 2882362..68ad494 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -834,6 +834,45 @@ static irqreturn_t exynos_dp_irq_handler(int irq, void *arg)
return IRQ_HANDLED;
}
+static void exynos_dp_hotplug(struct work_struct *work)
+{
+ struct exynos_dp_device *dp;
+ int ret;
+
+ dp = container_of(work, struct exynos_dp_device, hotplug_work);
+
+ ret = exynos_dp_detect_hpd(dp);
+ if (ret) {
+ dev_err(dp->dev, "unable to detect hpd\n");
+ return;
+ }
+
+ ret = exynos_dp_handle_edid(dp);
+ if (ret) {
+ dev_err(dp->dev, "unable to handle edid\n");
+ return;
+ }
+
+ ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
+ dp->video_info->link_rate);
+ if (ret) {
+ dev_err(dp->dev, "unable to do link train\n");
+ return;
+ }
+
+ exynos_dp_enable_scramble(dp, 1);
+ exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
+ exynos_dp_enable_enhanced_mode(dp, 1);
+
+ exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
+ exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
+
+ exynos_dp_init_video(dp);
+ ret = exynos_dp_config_video(dp, dp->video_info);
+ if (ret)
+ dev_err(&dp->dev, "unable to config video\n");
+}
+
static int __devinit exynos_dp_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -886,6 +925,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
goto err_clock;
}
+ INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
+
ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
"exynos-dp", dp);
if (ret) {
@@ -899,36 +940,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
exynos_dp_init_dp(dp);
- ret = exynos_dp_detect_hpd(dp);
- if (ret) {
- dev_err(&pdev->dev, "unable to detect hpd\n");
- goto err_clock;
- }
-
- exynos_dp_handle_edid(dp);
-
- ret = exynos_dp_set_link_train(dp, dp->video_info->lane_count,
- dp->video_info->link_rate);
- if (ret) {
- dev_err(&pdev->dev, "unable to do link train\n");
- goto err_clock;
- }
-
- exynos_dp_enable_scramble(dp, 1);
- exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
- exynos_dp_enable_enhanced_mode(dp, 1);
-
- exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
- exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
-
- exynos_dp_init_video(dp);
- ret = exynos_dp_config_video(dp, dp->video_info);
- if (ret) {
- dev_err(&pdev->dev, "unable to config video\n");
- goto err_clock;
- }
-
platform_set_drvdata(pdev, dp);
+ schedule_work(&dp->hotplug_work);
return 0;
@@ -943,6 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ if (work_pending(&dp->hotplug_work))
+ flush_work_sync(&dp->hotplug_work);
+
if (pdata && pdata->phy_exit)
pdata->phy_exit();
@@ -959,6 +975,9 @@ static int exynos_dp_suspend(struct device *dev)
struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
+ if (work_pending(&dp->hotplug_work))
+ flush_work_sync(&dp->hotplug_work);
+
if (pdata && pdata->phy_exit)
pdata->phy_exit();
@@ -979,22 +998,7 @@ static int exynos_dp_resume(struct device *dev)
clk_enable(dp->clock);
exynos_dp_init_dp(dp);
-
- exynos_dp_detect_hpd(dp);
- exynos_dp_handle_edid(dp);
-
- exynos_dp_set_link_train(dp, dp->video_info->lane_count,
- dp->video_info->link_rate);
-
- exynos_dp_enable_scramble(dp, 1);
- exynos_dp_enable_rx_to_enhanced_mode(dp, 1);
- exynos_dp_enable_enhanced_mode(dp, 1);
-
- exynos_dp_set_lane_count(dp, dp->video_info->lane_count);
- exynos_dp_set_link_bandwidth(dp, dp->video_info->link_rate);
-
- exynos_dp_init_video(dp);
- exynos_dp_config_video(dp, dp->video_info);
+ schedule_work(&dp->hotplug_work);
return 0;
}
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 6431c65..cf1010b 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -32,6 +32,7 @@ struct exynos_dp_device {
struct video_info *video_info;
struct link_train link_train;
+ struct work_struct hotplug_work;
};
/* exynos_dp_reg.c */
--
1.7.7.3
^ permalink raw reply related
* [PATCH 07/10] video: exynos_dp: Improve EDID error handling
From: Sean Paul @ 2012-08-08 3:54 UTC (permalink / raw)
To: linux-fbdev
EDID error handling has 2 problems:
- It doesn't fail as early as it can
- The retry counts for i2c and aux transactions are huge
This patch fails if the initial i2c transaction fails, and reduces the
aux and i2c retry counts down to 3.
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
---
drivers/video/exynos/exynos_dp_core.c | 18 ++++++++++--------
drivers/video/exynos/exynos_dp_reg.c | 9 ++++-----
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 1c998d9..2882362 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -89,9 +89,11 @@ static int exynos_dp_read_edid(struct exynos_dp_device *dp)
*/
/* Read Extension Flag, Number of 128-byte EDID extension blocks */
- exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
+ retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
EDID_EXTENSION_FLAG,
&extend_block);
+ if (retval)
+ return retval;
if (extend_block > 0) {
dev_dbg(dp->dev, "EDID data includes a single extension!\n");
@@ -177,21 +179,21 @@ static int exynos_dp_handle_edid(struct exynos_dp_device *dp)
{
u8 buf[12];
int i;
- int retval;
+ int ret;
/* Read DPCD DPCD_ADDR_DPCD_REV~RECEIVE_PORT1_CAP_1 */
- exynos_dp_read_bytes_from_dpcd(dp,
- DPCD_ADDR_DPCD_REV,
- 12, buf);
+ ret = exynos_dp_read_bytes_from_dpcd(dp, DPCD_ADDR_DPCD_REV, 12, buf);
+ if (ret)
+ return ret;
/* Read EDID */
for (i = 0; i < 3; i++) {
- retval = exynos_dp_read_edid(dp);
- if (retval = 0)
+ ret = exynos_dp_read_edid(dp);
+ if (!ret)
break;
}
- return retval;
+ return ret;
}
static void exynos_dp_enable_rx_to_enhanced_mode(struct exynos_dp_device *dp,
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index d7b1494..389e0f0 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -547,7 +547,7 @@ int exynos_dp_write_bytes_to_dpcd(struct exynos_dp_device *dp,
else
cur_data_count = count - start_offset;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -612,7 +612,7 @@ int exynos_dp_read_bytes_from_dpcd(struct exynos_dp_device *dp,
cur_data_count = count - start_offset;
/* AUX CH Request Transaction process */
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Select DPCD device address */
reg = AUX_ADDR_7_0(reg_addr + start_offset);
writel(reg, dp->reg_base + EXYNOS_DP_AUX_ADDR_7_0);
@@ -695,7 +695,7 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
int i;
int retval;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 3; i++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
@@ -703,7 +703,6 @@ int exynos_dp_read_byte_from_i2c(struct exynos_dp_device *dp,
/* Select EDID device */
retval = exynos_dp_select_i2c_device(dp, device_addr, reg_addr);
if (retval != 0) {
- dev_err(dp->dev, "Select EDID device fail!\n");
continue;
}
@@ -745,7 +744,7 @@ int exynos_dp_read_bytes_from_i2c(struct exynos_dp_device *dp,
int retval = 0;
for (i = 0; i < count; i += 16) {
- for (j = 0; j < 100; j++) {
+ for (j = 0; j < 3; j++) {
/* Clear AUX CH data buffer */
reg = BUF_CLR;
writel(reg, dp->reg_base + EXYNOS_DP_BUFFER_DATA_CTL);
--
1.7.7.3
^ permalink raw reply related
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