* Re: [PATCH] MXSFB: Fix driver registration
From: Shawn Guo @ 2011-12-19 13:51 UTC (permalink / raw)
To: Marek Vasut, Florian Tobias Schandinat
Cc: linux-kernel, Wolfgang Denk, Stefano Babic, Huang Shijie,
Axel Lin, Sascha Hauer, linux-fbdev
In-Reply-To: <1324169888-6687-1-git-send-email-marek.vasut@gmail.com>
Cc-ed Florian and linux-fbdev@vger.kernel.org, as I guess Florian is
the person who will take this patch.
-- Regards,Shawn
On 18 December 2011 08:58, Marek Vasut <marek.vasut@gmail.com> wrote:
> The driver should be registered with mxsfb_driver, not with mxsfb_devtype.
> This caused obvious null pointer dereference and crash.
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Huang Shijie <b32955@freescale.com>
> Cc: Axel Lin <axel.lin@gmail.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/video/mxsfb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 18742c2..d29c7c0 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -902,7 +902,7 @@ static struct platform_driver mxsfb_driver = {
> },
> };
>
> -module_platform_driver(mxsfb_devtype);
> +module_platform_driver(mxsfb_driver);
>
> MODULE_DESCRIPTION("Freescale mxs framebuffer driver");
> MODULE_AUTHOR("Sascha Hauer, Pengutronix");
> --
> 1.7.6.4
>
^ permalink raw reply
* Re: [GIT PULL] ARM: amba: Enable module alias autogeneration for
From: Dave Martin @ 2011-12-19 13:24 UTC (permalink / raw)
To: Alessandro Rubini
Cc: linux, patches, linux-kernel, linux-arm-kernel, alan, a.zummo,
alsa-devel, cjb, dan.j.williams, dmitry.torokhov, grant.likely,
perex, jassisinghbrar, julia, linus.walleij, linux-fbdev,
linux-input, linux-mmc, linux-serial, linux-watchdog, lethal,
Pawel.Moll, rtc-linux, spi-devel-general, tiwai, vinod.koul, wim
In-Reply-To: <20111206161815.GD13769@linaro.org>
Hi Russell,
This one isn't urgent, but I'm not seeing the amba modalias patches
anywhere yet. Did you have any outstanding concerns which need to be
resolved?
If you can suggest when/if these are likely to merge that would be great.
Since Alessandro is now expecting to have to rebase on top of the amba
additions anyway, we shouldn't need to worry about conflicting with his
patches.
Of course, if you've already merged these somewhere, then there's no
problem.
Cheers
---Dave
^ permalink raw reply
* [PATCH V2 2/2] OMAPDSS: DISPC: Update Scaling Clock Logic
From: Chandrabhanu Mahapatra @ 2011-12-19 8:45 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
Clock requirements for scaling in OMAP2, OMAP3 and OMAP4 are different. In
OMAP2 and OMAP3 the required clock rate is a function of pixel clock, vertical
downscale ratio and horizontal downscale ratio whereas in OMAP4 it is a
function of pixel clock and horizontal downscale ratio only. Selection of 3-tap
vs 5-tap coefficients depends on clock rate line buffer width in OMAP3 whereas
in OMAP4 it is independent of clock rate and line buffer width. In OMAP2 3-tap
for vertical and 5-tap for horizontal scaling is used. In OMAP4 5-tap is used
both for horizontal and vertical scaling for better performance. Also, the
number and width of line buffers differs in OMAP3 and OMAP4.
So, clock functions have been fined tuned for OMAP3 and support has been added
added for OMAP4. This code has been tested on OMAP2, OMAP3 and OMAP4, and
scaling issues due to clock errors have been resolved.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dispc.c | 66 ++++++++++++++++++++++----------
drivers/video/omap2/dss/dss_features.c | 7 +++
drivers/video/omap2/dss/dss_features.h | 1 +
3 files changed, 54 insertions(+), 20 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index b981983..6c3feb4 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1554,6 +1554,9 @@ static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
u32 fclk = 0;
u64 tmp, pclk = dispc_mgr_pclk_rate(channel);
+ if (height <= out_height && width <= out_width)
+ return (unsigned long) pclk;
+
if (height > out_height) {
struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
unsigned int ppl = dssdev->panel.timings.x_res;
@@ -1608,7 +1611,16 @@ static unsigned long calc_fclk(enum omap_channel channel, u16 width,
else
vf = 1;
- return dispc_mgr_pclk_rate(channel) * vf * hf;
+ if (cpu_is_omap24xx()) {
+ if (vf > 1 && hf > 1)
+ return dispc_mgr_pclk_rate(channel) * 4;
+ else
+ return dispc_mgr_pclk_rate(channel) * 2;
+ } else if (cpu_is_omap34xx()) {
+ return dispc_mgr_pclk_rate(channel) * vf * hf;
+ } else {
+ return dispc_mgr_pclk_rate(channel) * hf;
+ }
}
static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1618,6 +1630,8 @@ 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);
unsigned long fclk = 0;
if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0) {
@@ -1635,28 +1649,40 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
out_height > height * 8)
return -EINVAL;
- /* Must use 5-tap filter? */
- *five_taps = height > out_height * 2;
-
- if (!*five_taps) {
+ if (cpu_is_omap24xx()) {
+ if (width > maxsinglelinewidth)
+ DSSERR("Cannot scale max input width exceeded");
+ *five_taps = false;
+ fclk = calc_fclk(channel, width, height, out_width,
+ out_height);
+ } else if (cpu_is_omap34xx()) {
+ if (width > (maxsinglelinewidth * 2)) {
+ DSSERR("Cannot setup scaling");
+ DSSERR("width exceeds maximum width possible");
+ return -EINVAL;
+ }
+ fclk = calc_fclk_five_taps(channel, width, height, out_width,
+ out_height, color_mode);
+ if (width > maxsinglelinewidth) {
+ if (height > out_height && height < out_height * 2)
+ *five_taps = false;
+ else {
+ DSSERR("cannot setup scaling with five taps");
+ return -EINVAL;
+ }
+ }
+ if (!*five_taps)
+ fclk = calc_fclk(channel, width, height, out_width,
+ out_height);
+ } else {
+ if (width > maxsinglelinewidth) {
+ DSSERR("Cannot scale width exceeds max line width");
+ return -EINVAL;
+ }
fclk = calc_fclk(channel, width, height, out_width,
out_height);
-
- /* Try 5-tap filter if 3-tap fclk is too high */
- if (cpu_is_omap34xx() && height > out_height &&
- fclk > dispc_fclk_rate())
- *five_taps = true;
}
- if (width > (2048 >> *five_taps)) {
- DSSERR("failed to set up scaling, fclk too low\n");
- return -EINVAL;
- }
-
- if (*five_taps)
- fclk = calc_fclk_five_taps(channel, width, height,
- out_width, out_height, color_mode);
-
DSSDBG("required fclk rate = %lu Hz\n", fclk);
DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
@@ -1676,7 +1702,7 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
u32 fifo_low, u32 fifo_high)
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
- bool five_taps = false;
+ bool five_taps = true;
bool fieldmode = 0;
int r, cconv = 0;
unsigned offset0, offset1;
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index b402699..5e4b829 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -304,6 +304,11 @@ static const struct dss_param_range omap2_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 0, 0 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 0, 0 },
[FEAT_PARAM_DOWNSCALE] = { 1, 2 },
+ /*
+ * Assuming the line width buffer to be 768 pixels as OMAP2 DISPC
+ * scaler cannot scale a image with width more than 768.
+ */
+ [FEAT_PARAM_LINEWIDTH] = { 1, 768 },
};
static const struct dss_param_range omap3_dss_param_range[] = {
@@ -316,6 +321,7 @@ static const struct dss_param_range omap3_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 750000, 2100000 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 1, (1 << 13) - 1},
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
+ [FEAT_PARAM_LINEWIDTH] = { 1, 1024 },
};
static const struct dss_param_range omap4_dss_param_range[] = {
@@ -328,6 +334,7 @@ static const struct dss_param_range omap4_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 500000, 2500000 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 0, (1 << 13) - 1 },
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
+ [FEAT_PARAM_LINEWIDTH] = { 1, 2048 },
};
/* OMAP2 DSS Features */
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index 6a6c05d..cd833bb 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -86,6 +86,7 @@ enum dss_range_param {
FEAT_PARAM_DSIPLL_FINT,
FEAT_PARAM_DSIPLL_LPDIV,
FEAT_PARAM_DOWNSCALE,
+ FEAT_PARAM_LINEWIDTH,
};
/* DSS Feature Functions */
--
1.7.1
^ permalink raw reply related
* [PATCH V2 1/2] OMAPDSS: DISPC: Update Fir Coefficients
From: Chandrabhanu Mahapatra @ 2011-12-19 8:45 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
The FIR coefficients present in kernel are being updated to new coefficients
consisting of 24 coefficient tables, with 12 each for 3 tap and 5 tap scenario,
which are chosen on the basis of DISPC up/downsampling filters M value. M is
the inverse of low pass cut off frequency of the sampling filter. For vertical
scaling 3 tap or 5 tap tables are used based on the clock rate and width of
the line buffer whereas in OMAP2 3 tap is always used. For horizontal scaling
however 5 tap tables are always used.
New coefficients and the corresponding logic have been tested on OMAP2, OMAP3
and OMAP4. Horizontal and vertical scaling worked fine except for some 3 tap
vs 5 tap issue during vertical upscaling and clock failing issues which is
acknowledged in the next patch. Vertical upscaling was found to perform better
under 5 taps. The 24 coefficient tables have been moved to another file
dispc_coefs.c for proper maintainance.
This code is written based on code written by Lajos Molnar <lajos@ti.com> in
Android Kernel for scaling. Lajos Molnar <lajos@ti.com> had fine tuned the FIR
coefficient selection process and reduced outliness and blockiness around
images when upscaling more than 2 times.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/Makefile | 2 +-
drivers/video/omap2/dss/dispc.c | 135 ++------------
drivers/video/omap2/dss/dispc.h | 11 +
drivers/video/omap2/dss/dispc_coefs.c | 326 +++++++++++++++++++++++++++++++++
4 files changed, 356 insertions(+), 118 deletions(-)
create mode 100644 drivers/video/omap2/dss/dispc_coefs.c
diff --git a/drivers/video/omap2/dss/Makefile b/drivers/video/omap2/dss/Makefile
index bd34ac5..f10d56f 100644
--- a/drivers/video/omap2/dss/Makefile
+++ b/drivers/video/omap2/dss/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_OMAP2_DSS) += omapdss.o
-omapdss-y := core.o dss.o dss_features.o dispc.o display.o manager.o overlay.o
+omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o manager.o overlay.o
omapdss-$(CONFIG_OMAP2_DSS_DPI) += dpi.o
omapdss-$(CONFIG_OMAP2_DSS_RFBI) += rfbi.o
omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 6892cfd..b981983 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -63,22 +63,6 @@ struct omap_dispc_isr_data {
u32 mask;
};
-struct dispc_h_coef {
- s8 hc4;
- s8 hc3;
- u8 hc2;
- s8 hc1;
- s8 hc0;
-};
-
-struct dispc_v_coef {
- s8 vc22;
- s8 vc2;
- u8 vc1;
- s8 vc0;
- s8 vc00;
-};
-
enum omap_burst_size {
BURST_SIZE_X2 = 0,
BURST_SIZE_X4 = 1,
@@ -532,105 +516,27 @@ static void dispc_ovl_write_firv2_reg(enum omap_plane plane, int reg, u32 value)
dispc_write_reg(DISPC_OVL_FIR_COEF_V2(plane, reg), value);
}
-static void dispc_ovl_set_scale_coef(enum omap_plane plane, int hscaleup,
- int vscaleup, int five_taps,
- enum omap_color_component color_comp)
-{
- /* Coefficients for horizontal up-sampling */
- static const struct dispc_h_coef coef_hup[8] = {
- { 0, 0, 128, 0, 0 },
- { -1, 13, 124, -8, 0 },
- { -2, 30, 112, -11, -1 },
- { -5, 51, 95, -11, -2 },
- { 0, -9, 73, 73, -9 },
- { -2, -11, 95, 51, -5 },
- { -1, -11, 112, 30, -2 },
- { 0, -8, 124, 13, -1 },
- };
-
- /* Coefficients for vertical up-sampling */
- static const struct dispc_v_coef coef_vup_3tap[8] = {
- { 0, 0, 128, 0, 0 },
- { 0, 3, 123, 2, 0 },
- { 0, 12, 111, 5, 0 },
- { 0, 32, 89, 7, 0 },
- { 0, 0, 64, 64, 0 },
- { 0, 7, 89, 32, 0 },
- { 0, 5, 111, 12, 0 },
- { 0, 2, 123, 3, 0 },
- };
-
- static const struct dispc_v_coef coef_vup_5tap[8] = {
- { 0, 0, 128, 0, 0 },
- { -1, 13, 124, -8, 0 },
- { -2, 30, 112, -11, -1 },
- { -5, 51, 95, -11, -2 },
- { 0, -9, 73, 73, -9 },
- { -2, -11, 95, 51, -5 },
- { -1, -11, 112, 30, -2 },
- { 0, -8, 124, 13, -1 },
- };
-
- /* Coefficients for horizontal down-sampling */
- static const struct dispc_h_coef coef_hdown[8] = {
- { 0, 36, 56, 36, 0 },
- { 4, 40, 55, 31, -2 },
- { 8, 44, 54, 27, -5 },
- { 12, 48, 53, 22, -7 },
- { -9, 17, 52, 51, 17 },
- { -7, 22, 53, 48, 12 },
- { -5, 27, 54, 44, 8 },
- { -2, 31, 55, 40, 4 },
- };
-
- /* Coefficients for vertical down-sampling */
- static const struct dispc_v_coef coef_vdown_3tap[8] = {
- { 0, 36, 56, 36, 0 },
- { 0, 40, 57, 31, 0 },
- { 0, 45, 56, 27, 0 },
- { 0, 50, 55, 23, 0 },
- { 0, 18, 55, 55, 0 },
- { 0, 23, 55, 50, 0 },
- { 0, 27, 56, 45, 0 },
- { 0, 31, 57, 40, 0 },
- };
-
- static const struct dispc_v_coef coef_vdown_5tap[8] = {
- { 0, 36, 56, 36, 0 },
- { 4, 40, 55, 31, -2 },
- { 8, 44, 54, 27, -5 },
- { 12, 48, 53, 22, -7 },
- { -9, 17, 52, 51, 17 },
- { -7, 22, 53, 48, 12 },
- { -5, 27, 54, 44, 8 },
- { -2, 31, 55, 40, 4 },
- };
-
- const struct dispc_h_coef *h_coef;
- const struct dispc_v_coef *v_coef;
+static void dispc_ovl_set_scale_coef(enum omap_plane plane, int fir_hinc,
+ int fir_vinc, int five_taps,
+ enum omap_color_component color_comp)
+{
+ const struct dispc_coef *h_coef, *v_coef;
int i;
- if (hscaleup)
- h_coef = coef_hup;
- else
- h_coef = coef_hdown;
-
- if (vscaleup)
- v_coef = five_taps ? coef_vup_5tap : coef_vup_3tap;
- else
- v_coef = five_taps ? coef_vdown_5tap : coef_vdown_3tap;
+ h_coef = dispc_ovl_get_scale_coef(fir_hinc, true);
+ v_coef = dispc_ovl_get_scale_coef(fir_vinc, five_taps);
for (i = 0; i < 8; i++) {
u32 h, hv;
- h = FLD_VAL(h_coef[i].hc0, 7, 0)
- | FLD_VAL(h_coef[i].hc1, 15, 8)
- | FLD_VAL(h_coef[i].hc2, 23, 16)
- | FLD_VAL(h_coef[i].hc3, 31, 24);
- hv = FLD_VAL(h_coef[i].hc4, 7, 0)
- | FLD_VAL(v_coef[i].vc0, 15, 8)
- | FLD_VAL(v_coef[i].vc1, 23, 16)
- | FLD_VAL(v_coef[i].vc2, 31, 24);
+ h = FLD_VAL(h_coef[i].hc0_vc00, 7, 0)
+ | FLD_VAL(h_coef[i].hc1_vc0, 15, 8)
+ | FLD_VAL(h_coef[i].hc2_vc1, 23, 16)
+ | FLD_VAL(h_coef[i].hc3_vc2, 31, 24);
+ hv = FLD_VAL(h_coef[i].hc4_vc22, 7, 0)
+ | FLD_VAL(v_coef[i].hc1_vc0, 15, 8)
+ | FLD_VAL(v_coef[i].hc2_vc1, 23, 16)
+ | FLD_VAL(v_coef[i].hc3_vc2, 31, 24);
if (color_comp = DISPC_COLOR_COMPONENT_RGB_Y) {
dispc_ovl_write_firh_reg(plane, i, h);
@@ -645,8 +551,8 @@ static void dispc_ovl_set_scale_coef(enum omap_plane plane, int hscaleup,
if (five_taps) {
for (i = 0; i < 8; i++) {
u32 v;
- v = FLD_VAL(v_coef[i].vc00, 7, 0)
- | FLD_VAL(v_coef[i].vc22, 15, 8);
+ v = FLD_VAL(v_coef[i].hc0_vc00, 7, 0)
+ | FLD_VAL(v_coef[i].hc4_vc22, 15, 8);
if (color_comp = DISPC_COLOR_COMPONENT_RGB_Y)
dispc_ovl_write_firv_reg(plane, i, v);
else
@@ -1168,17 +1074,12 @@ static void dispc_ovl_set_scale_param(enum omap_plane plane,
enum omap_color_component color_comp)
{
int fir_hinc, fir_vinc;
- int hscaleup, vscaleup;
-
- hscaleup = orig_width <= out_width;
- vscaleup = orig_height <= out_height;
-
- dispc_ovl_set_scale_coef(plane, hscaleup, vscaleup, five_taps,
- color_comp);
fir_hinc = 1024 * orig_width / out_width;
fir_vinc = 1024 * orig_height / out_height;
+ dispc_ovl_set_scale_coef(plane, fir_hinc, fir_vinc, five_taps,
+ color_comp);
dispc_ovl_set_fir(plane, fir_hinc, fir_vinc, color_comp);
}
diff --git a/drivers/video/omap2/dss/dispc.h b/drivers/video/omap2/dss/dispc.h
index c06efc3..5836bd1 100644
--- a/drivers/video/omap2/dss/dispc.h
+++ b/drivers/video/omap2/dss/dispc.h
@@ -97,6 +97,17 @@
#define DISPC_OVL_PRELOAD(n) (DISPC_OVL_BASE(n) + \
DISPC_PRELOAD_OFFSET(n))
+/* DISPC up/downsampling FIR filter coefficient structure */
+struct dispc_coef {
+ s8 hc4_vc22;
+ s8 hc3_vc2;
+ u8 hc2_vc1;
+ s8 hc1_vc0;
+ s8 hc0_vc00;
+};
+
+const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps);
+
/* DISPC manager/channel specific registers */
static inline u16 DISPC_DEFAULT_COLOR(enum omap_channel channel)
{
diff --git a/drivers/video/omap2/dss/dispc_coefs.c b/drivers/video/omap2/dss/dispc_coefs.c
new file mode 100644
index 0000000..069bccb
--- /dev/null
+++ b/drivers/video/omap2/dss/dispc_coefs.c
@@ -0,0 +1,326 @@
+/*
+ * linux/drivers/video/omap2/dss/dispc_coefs.c
+ *
+ * Copyright (C) 2011 Texas Instruments
+ * Author: Chandrabhanu Mahapatra <cmahapatra@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <video/omapdss.h>
+#include "dispc.h"
+
+#define ARRAY_LEN(array) (sizeof(array) / sizeof(array[0]))
+
+static const struct dispc_coef coef3_M8[8] = {
+ { 0, 0, 128, 0, 0 },
+ { 0, -4, 123, 9, 0 },
+ { 0, -4, 108, 87, 0 },
+ { 0, -2, 87, 43, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 43, 87, -2, 0 },
+ { 0, 24, 108, -4, 0 },
+ { 0, 9, 123, -4, 0 },
+};
+
+static const struct dispc_coef coef3_M9[8] = {
+ { 0, 6, 116, 6, 0 },
+ { 0, 0, 112, 16, 0 },
+ { 0, -2, 100, 30, 0 },
+ { 0, -2, 83, 47, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 47, 83, -2, 0 },
+ { 0, 30, 100, -2, 0 },
+ { 0, 16, 112, 0, 0 },
+};
+
+static const struct dispc_coef coef3_M10[8] = {
+ { 0, 10, 108, 10, 0 },
+ { 0, 3, 104, 21, 0 },
+ { 0, 0, 94, 34, 0 },
+ { 0, -1, 80, 49, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 49, 80, -1, 0 },
+ { 0, 34, 94, 0, 0 },
+ { 0, 21, 104, 3, 0 },
+};
+
+static const struct dispc_coef coef3_M11[8] = {
+ { 0, 14, 100, 14, 0 },
+ { 0, 6, 98, 24, 0 },
+ { 0, 2, 90, 36, 0 },
+ { 0, 0, 78, 50, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 50, 78, 0, 0 },
+ { 0, 36, 90, 2, 0 },
+ { 0, 24, 98, 6, 0 },
+};
+
+static const struct dispc_coef coef3_M12[8] = {
+ { 0, 16, 96, 16, 0 },
+ { 0, 9, 93, 26, 0 },
+ { 0, 4, 86, 38, 0 },
+ { 0, 1, 76, 51, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 51, 76, 1, 0 },
+ { 0, 38, 86, 4, 0 },
+ { 0, 26, 93, 9, 0 },
+};
+
+static const struct dispc_coef coef3_M13[8] = {
+ { 0, 18, 92, 18, 0 },
+ { 0, 10, 90, 28, 0 },
+ { 0, 5, 83, 40, 0 },
+ { 0, 1, 75, 52, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 52, 75, 1, 0 },
+ { 0, 40, 83, 5, 0 },
+ { 0, 28, 90, 10, 0 },
+};
+
+static const struct dispc_coef coef3_M14[8] = {
+ { 0, 20, 88, 20, 0 },
+ { 0, 12, 86, 30, 0 },
+ { 0, 6, 81, 41, 0 },
+ { 0, 2, 74, 52, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 52, 74, 2, 0 },
+ { 0, 41, 81, 6, 0 },
+ { 0, 30, 86, 12, 0 },
+};
+
+static const struct dispc_coef coef3_M16[8] = {
+ { 0, 22, 84, 22, 0 },
+ { 0, 14, 82, 32, 0 },
+ { 0, 8, 78, 42, 0 },
+ { 0, 3, 72, 53, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 53, 72, 3, 0 },
+ { 0, 42, 78, 8, 0 },
+ { 0, 32, 82, 14, 0 },
+};
+
+static const struct dispc_coef coef3_M19[8] = {
+ { 0, 24, 80, 24, 0 },
+ { 0, 16, 79, 33, 0 },
+ { 0, 9, 76, 43, 0 },
+ { 0, 4, 70, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 70, 4, 0 },
+ { 0, 43, 76, 9, 0 },
+ { 0, 33, 79, 16, 0 },
+};
+
+static const struct dispc_coef coef3_M22[8] = {
+ { 0, 25, 78, 25, 0 },
+ { 0, 17, 77, 34, 0 },
+ { 0, 10, 74, 44, 0 },
+ { 0, 5, 69, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 69, 5, 0 },
+ { 0, 44, 74, 10, 0 },
+ { 0, 34, 77, 17, 0 },
+};
+
+static const struct dispc_coef coef3_M26[8] = {
+ { 0, 26, 76, 26, 0 },
+ { 0, 19, 74, 35, 0 },
+ { 0, 11, 72, 45, 0 },
+ { 0, 5, 69, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 69, 5, 0 },
+ { 0, 45, 72, 11, 0 },
+ { 0, 35, 74, 19, 0 },
+};
+
+static const struct dispc_coef coef3_M32[8] = {
+ { 0, 27, 74, 27, 0 },
+ { 0, 19, 73, 36, 0 },
+ { 0, 12, 71, 45, 0 },
+ { 0, 6, 68, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 68, 6, 0 },
+ { 0, 45, 71, 12, 0 },
+ { 0, 36, 73, 19, 0 },
+};
+
+static const struct dispc_coef coef5_M8[8] = {
+ { 0, 0, 128, 0, 0 },
+ { -2, 14, 125, -10, 1 },
+ { -6, 33, 114, -15, 2 },
+ { -10, 55, 98, -16, 1 },
+ { 0, -14, 78, 78, -14 },
+ { 1, -16, 98, 55, -10 },
+ { 2, -15, 114, 33, -6 },
+ { 1, -10, 125, 14, -2 },
+};
+
+static const struct dispc_coef coef5_M9[8] = {
+ { -3, 10, 114, 10, -3 },
+ { -6, 24, 110, 0, -1 },
+ { -8, 40, 103, -7, 0 },
+ { -11, 58, 91, -11, 1 },
+ { 0, -12, 76, 76, -12 },
+ { 1, -11, 91, 58, -11 },
+ { 0, -7, 103, 40, -8 },
+ { -1, 0, 111, 24, -6 },
+};
+
+static const struct dispc_coef coef5_M10[8] = {
+ { -4, 18, 100, 18, -4 },
+ { -6, 30, 99, 8, -3 },
+ { -8, 44, 93, 0, -1 },
+ { -9, 58, 84, -5, 0 },
+ { 0, -8, 72, 72, -8 },
+ { 0, -5, 84, 58, -9 },
+ { -1, 0, 93, 44, -8 },
+ { -3, 8, 99, 30, -6 },
+};
+
+static const struct dispc_coef coef5_M11[8] = {
+ { -5, 23, 92, 23, -5 },
+ { -6, 34, 90, 13, -3 },
+ { -6, 45, 85, 6, -2 },
+ { -6, 57, 78, 0, -1 },
+ { 0, -4, 68, 68, -4 },
+ { -1, 0, 78, 57, -6 },
+ { -2, 6, 85, 45, -6 },
+ { -3, 13, 90, 34, -6 },
+};
+
+static const struct dispc_coef coef5_M12[8] = {
+ { -4, 26, 84, 26, -4 },
+ { -5, 36, 82, 18, -3 },
+ { -4, 46, 78, 10, -2 },
+ { -3, 55, 72, 5, -1 },
+ { 0, 0, 64, 64, 0 },
+ { -1, 5, 72, 55, -3 },
+ { -2, 10, 78, 46, -4 },
+ { -3, 18, 82, 36, -5 },
+};
+
+static const struct dispc_coef coef5_M13[8] = {
+ { -3, 28, 78, 28, -3 },
+ { -3, 37, 76, 21, -3 },
+ { -2, 45, 73, 14, -2 },
+ { 0, 53, 68, 8, -1 },
+ { 0, 3, 61, 61, 3 },
+ { -1, 8, 68, 53, 0 },
+ { -2, 14, 73, 45, -2 },
+ { -3, 21, 76, 37, -3 },
+};
+
+static const struct dispc_coef coef5_M14[8] = {
+ { -2, 30, 72, 30, -2 },
+ { -1, 37, 71, 23, -2 },
+ { 0, 45, 69, 16, -2 },
+ { 3, 52, 64, 10, -1 },
+ { 0, 6, 58, 58, 6 },
+ { -1, 10, 64, 52, 3 },
+ { -2, 16, 69, 45, 0 },
+ { -2, 23, 71, 37, -1 },
+};
+
+static const struct dispc_coef coef5_M16[8] = {
+ { 0, 31, 66, 31, 0 },
+ { 1, 38, 65, 25, -1 },
+ { 3, 44, 62, 20, -1 },
+ { 6, 49, 59, 14, 0 },
+ { 0, 10, 54, 54, 10 },
+ { 0, 14, 59, 49, 6 },
+ { -1, 20, 62, 44, 3 },
+ { -1, 25, 65, 38, 1 },
+};
+
+static const struct dispc_coef coef5_M19[8] = {
+ { 3, 32, 58, 32, 3 },
+ { 4, 38, 58, 27, 1 },
+ { 7, 42, 55, 23, 1 },
+ { 10, 46, 54, 18, 0 },
+ { 0, 14, 50, 50, 14 },
+ { 0, 18, 54, 46, 10 },
+ { 1, 23, 55, 42, 7 },
+ { 1, 27, 58, 38, 4 },
+};
+
+static const struct dispc_coef coef5_M22[8] = {
+ { 4, 33, 54, 33, 4 },
+ { 6, 37, 54, 28, 3 },
+ { 9, 41, 53, 24, 1 },
+ { 12, 45, 51, 20, 0 },
+ { 0, 16, 48, 48, 16 },
+ { 0, 20, 51, 45, 12 },
+ { 1, 24, 53, 41, 9 },
+ { 3, 28, 54, 37, 6 },
+};
+
+static const struct dispc_coef coef5_M26[8] = {
+ { 6, 33, 50, 33, 6 },
+ { 8, 36, 51, 29, 4 },
+ { 11, 40, 50, 25, 2 },
+ { 14, 43, 48, 22, 1 },
+ { 0, 18, 46, 46, 18 },
+ { 1, 22, 48, 43, 14 },
+ { 2, 25, 50, 40, 11 },
+ { 4, 29, 51, 36, 8 },
+};
+
+static const struct dispc_coef coef5_M32[8] = {
+ { 7, 33, 48, 33, 7 },
+ { 10, 36, 48, 29, 5 },
+ { 13, 39, 47, 26, 3 },
+ { 16, 42, 46, 23, 1 },
+ { 0, 19, 45, 45, 19 },
+ { 1, 23, 46, 42, 16 },
+ { 3, 26, 47, 39, 13 },
+ { 5, 29, 48, 36, 10 },
+};
+
+const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps)
+{
+ int i;
+ static const struct {
+ int Mmin;
+ int Mmax;
+ const struct dispc_coef *coef_3;
+ const struct dispc_coef *coef_5;
+ } coefs[] = {
+ { 27, 32, coef3_M32, coef5_M32 },
+ { 23, 26, coef3_M26, coef5_M26 },
+ { 20, 22, coef3_M22, coef5_M22 },
+ { 17, 19, coef3_M19, coef5_M19 },
+ { 15, 16, coef3_M16, coef5_M16 },
+ { 14, 14, coef3_M14, coef5_M14 },
+ { 13, 13, coef3_M13, coef5_M13 },
+ { 12, 12, coef3_M12, coef5_M12 },
+ { 11, 11, coef3_M11, coef5_M11 },
+ { 10, 10, coef3_M10, coef5_M10 },
+ { 9, 9, coef3_M9, coef5_M9 },
+ { 4, 8, coef3_M8, coef5_M8 },
+ /*
+ * When upscaling more than two times, blockiness and outlines
+ * around the image are observed when M8 tables are used. M11,
+ * M16 and M19 tables are used to prevent this.
+ */
+ { 3, 3, coef3_M11, coef5_M11 },
+ { 2, 2, coef3_M16, coef5_M16 },
+ { 0, 1, coef3_M19, coef5_M19 },
+ };
+
+ inc /= 128;
+ for (i = 0; i < ARRAY_LEN(coefs); ++i)
+ if (inc >= coefs[i].Mmin && inc <= coefs[i].Mmax)
+ return five_taps ? coefs[i].coef_5 : coefs[i].coef_3;
+ return NULL;
+}
--
1.7.1
^ permalink raw reply related
* [PATCH V2 0/2] OMAPDSS: DISPC: Peformance improvement of DISPC Scaling
From: Chandrabhanu Mahapatra @ 2011-12-19 8:45 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
Hi everyone,
the following patch set directs to improve the scaling performance of DISPC
module which consists of two pacthes.
The first patch is based on code of Lajos Molnar <lajos@ti.com> from Android
Kernel, which updates the code with new set of coefficients to improve the
scaling performance.
The second patch is based on some very valuable suggestions by
Sebastien Fagard <s-fagard@ti.com> and directs to modify the clock
requirements for scaling to support OMAP4 and avoid clock failure issues.
I have tested these patches on OMAP2, OMAP3 AND OMAP4. To test on 2430SDP
board I have created a patch to enable the display which will follow later.
All your comments and suggestions are welcome.
Regards,
Chandrabhanu
Chandrabhanu Mahapatra (2):
OMAPDSS: DISPC: Update Fir Coefficients
OMAPDSS: DISPC: Update Scaling Clock Logic
drivers/video/omap2/dss/Makefile | 2 +-
drivers/video/omap2/dss/dispc.c | 203 +++++++--------------
drivers/video/omap2/dss/dispc.h | 11 +
drivers/video/omap2/dss/dispc_coefs.c | 326 ++++++++++++++++++++++++++++++++
drivers/video/omap2/dss/dss_features.c | 7 +
drivers/video/omap2/dss/dss_features.h | 1 +
6 files changed, 411 insertions(+), 139 deletions(-)
create mode 100644 drivers/video/omap2/dss/dispc_coefs.c
^ permalink raw reply
* RE: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Jingoo Han @ 2011-12-19 3:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
Hi Florian,
Florian Tobias Schandinat wrote:
> -----Original Message-----
> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
>
> On 12/19/2011 02:19 AM, Jingoo Han wrote:
> > Hi Florian,
> >
> > Florian Tobias Schandinat wrote:
> >> -----Original Message-----
> >> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
> >>
> >> Hi Jingoo,
> >>
> >> On 12/05/2011 02:42 AM, Jingoo Han wrote:
> >>> Runtime suspend and runtime resume are modified in order to
> >>> reduce the complexity and improve the usability of runtime pm.
> >>> After probe function, s3c-fb driver is not suspended until
> >>> suspend or remove is called.
> >>>
> >>> The scheme is changed as follows:
> >>> runtime_get is only called in probe and resume.
> >>> runtime_put is only called in remove and suspend.
> >>> open/close cannot call the runtime_get/put.
> >>>
> >>> Also, runtime_susepnd/resume are just called by runtime pm,
> >>> not doing suspend/resume routine any longer. This is because
> >>> open/close cannot call the runtime_get/put; the suspend/resume
> >>> routine in runtime_suspend/resume were previously used when
> >>> open and close were called.
> >>>
> >>> The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
> >>> s3c_fb_pm_ops in order to use more consistent naming.
> >>>
> >>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> >>
> >> the patch itself looks okay to me, but I'm wondering what changes will be
> >> visible to the user, if any?
> >> Does the behavior change for example, when no one has called open?
> > Previously, in order to display framebuffer to LCD panel, user should call
> > open after booting, because runtime_pm_put is called after prove function.
> >
> > However, now, after booting, framebuffer can be displayed to LCD panel
> > without calling open.
>
> As I thought. What is displayed in such a case? (no open called = no one put any
> image in the framebuffer)
Sorry, user should call open, too. There is no visible change for user.
Just runtime pm put & get are removed between probe and open.
> As the new behavior is probably okay for single framebuffer I'll apply this
> patch, I'm just asking as I'm planning to do quite the opposite in viafb to get
> rid of this situation in dual framebuffer mode where it is quite likely that one
> display is not used for some time and at the moment only displays rubbish.
OK, I'll check dual framebuffer mode.
Thank you.
Best regards,
Jingoo Han
>
> Best regards,
>
> Florian Tobias Schandinat
>
> >
> > Best regards,
> > Jingoo Han
> >>
> >>
> >> Best regards,
> >>
> >> Florian Tobias Schandinat
> >>
> >>> ---
> >>> v2: fix unaligned lines
> >>>
> >>> drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
> >>> 1 files changed, 20 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> >>> index e84677e..91e629a 100644
> >>> --- a/drivers/video/s3c-fb.c
> >>> +++ b/drivers/video/s3c-fb.c
> >>> @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
> >>> return ret;
> >>> }
> >>>
> >>> -static int s3c_fb_open(struct fb_info *info, int user)
> >>> -{
> >>> - struct s3c_fb_win *win = info->par;
> >>> - struct s3c_fb *sfb = win->parent;
> >>> -
> >>> - pm_runtime_get_sync(sfb->dev);
> >>> -
> >>> - return 0;
> >>> -}
> >>> -
> >>> -static int s3c_fb_release(struct fb_info *info, int user)
> >>> -{
> >>> - struct s3c_fb_win *win = info->par;
> >>> - struct s3c_fb *sfb = win->parent;
> >>> -
> >>> - pm_runtime_put_sync(sfb->dev);
> >>> -
> >>> - return 0;
> >>> -}
> >>> -
> >>> static struct fb_ops s3c_fb_ops = {
> >>> .owner = THIS_MODULE,
> >>> - .fb_open = s3c_fb_open,
> >>> - .fb_release = s3c_fb_release,
> >>> .fb_check_var = s3c_fb_check_var,
> >>> .fb_set_par = s3c_fb_set_par,
> >>> .fb_blank = s3c_fb_blank,
> >>> @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> >>> }
> >>>
> >>> platform_set_drvdata(pdev, sfb);
> >>> - pm_runtime_put_sync(sfb->dev);
> >>>
> >>> return 0;
> >>>
> >>> @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> >>> struct s3c_fb *sfb = platform_get_drvdata(pdev);
> >>> int win;
> >>>
> >>> - pm_runtime_get_sync(sfb->dev);
> >>> -
> >>> for (win = 0; win < S3C_FB_MAX_WIN; win++)
> >>> if (sfb->windows[win])
> >>> s3c_fb_release_win(sfb, sfb->windows[win]);
> >>> @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> >>> return 0;
> >>> }
> >>>
> >>> -#ifdef CONFIG_PM
> >>> +#ifdef CONFIG_PM_SLEEP
> >>> static int s3c_fb_suspend(struct device *dev)
> >>> {
> >>> struct platform_device *pdev = to_platform_device(dev);
> >>> @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
> >>> clk_disable(sfb->lcd_clk);
> >>>
> >>> clk_disable(sfb->bus_clk);
> >>> + pm_runtime_put_sync(sfb->dev);
> >>> +
> >>> return 0;
> >>> }
> >>>
> >>> @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
> >>> struct s3c_fb_win *win;
> >>> int win_no;
> >>>
> >>> + pm_runtime_get_sync(sfb->dev);
> >>> clk_enable(sfb->bus_clk);
> >>>
> >>> if (!sfb->variant.has_clksel)
> >>> @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
> >>>
> >>> return 0;
> >>> }
> >>> -#else
> >>> -#define s3c_fb_suspend NULL
> >>> -#define s3c_fb_resume NULL
> >>> #endif
> >>>
> >>> +#ifdef CONFIG_PM_RUNTIME
> >>> +static int s3c_fb_runtime_suspend(struct device *dev)
> >>> +{
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int s3c_fb_runtime_resume(struct device *dev)
> >>> +{
> >>> + return 0;
> >>> +}
> >>> +#endif
> >>>
> >>> #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
> >>> #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
> >>> @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> >>> };
> >>> MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
> >>>
> >>> -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
> >>> +static const struct dev_pm_ops s3c_fb_pm_ops = {
> >>> + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
> >>> + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
> >>> +};
> >>>
> >>> static struct platform_driver s3c_fb_driver = {
> >>> .probe = s3c_fb_probe,
> >>> @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
> >>> .driver = {
> >>> .name = "s3c-fb",
> >>> .owner = THIS_MODULE,
> >>> - .pm = &s3cfb_pm_ops,
> >>> + .pm = &s3c_fb_pm_ops,
> >>> },
> >>> };
> >>>
> >
> >
^ permalink raw reply
* Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Florian Tobias Schandinat @ 2011-12-19 2:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
On 12/19/2011 02:19 AM, Jingoo Han wrote:
> Hi Florian,
>
> Florian Tobias Schandinat wrote:
>> -----Original Message-----
>> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
>>
>> Hi Jingoo,
>>
>> On 12/05/2011 02:42 AM, Jingoo Han wrote:
>>> Runtime suspend and runtime resume are modified in order to
>>> reduce the complexity and improve the usability of runtime pm.
>>> After probe function, s3c-fb driver is not suspended until
>>> suspend or remove is called.
>>>
>>> The scheme is changed as follows:
>>> runtime_get is only called in probe and resume.
>>> runtime_put is only called in remove and suspend.
>>> open/close cannot call the runtime_get/put.
>>>
>>> Also, runtime_susepnd/resume are just called by runtime pm,
>>> not doing suspend/resume routine any longer. This is because
>>> open/close cannot call the runtime_get/put; the suspend/resume
>>> routine in runtime_suspend/resume were previously used when
>>> open and close were called.
>>>
>>> The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
>>> s3c_fb_pm_ops in order to use more consistent naming.
>>>
>>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>>
>> the patch itself looks okay to me, but I'm wondering what changes will be
>> visible to the user, if any?
>> Does the behavior change for example, when no one has called open?
> Previously, in order to display framebuffer to LCD panel, user should call
> open after booting, because runtime_pm_put is called after prove function.
>
> However, now, after booting, framebuffer can be displayed to LCD panel
> without calling open.
As I thought. What is displayed in such a case? (no open called = no one put any
image in the framebuffer)
As the new behavior is probably okay for single framebuffer I'll apply this
patch, I'm just asking as I'm planning to do quite the opposite in viafb to get
rid of this situation in dual framebuffer mode where it is quite likely that one
display is not used for some time and at the moment only displays rubbish.
Best regards,
Florian Tobias Schandinat
>
> Best regards,
> Jingoo Han
>>
>>
>> Best regards,
>>
>> Florian Tobias Schandinat
>>
>>> ---
>>> v2: fix unaligned lines
>>>
>>> drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
>>> 1 files changed, 20 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
>>> index e84677e..91e629a 100644
>>> --- a/drivers/video/s3c-fb.c
>>> +++ b/drivers/video/s3c-fb.c
>>> @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>> return ret;
>>> }
>>>
>>> -static int s3c_fb_open(struct fb_info *info, int user)
>>> -{
>>> - struct s3c_fb_win *win = info->par;
>>> - struct s3c_fb *sfb = win->parent;
>>> -
>>> - pm_runtime_get_sync(sfb->dev);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static int s3c_fb_release(struct fb_info *info, int user)
>>> -{
>>> - struct s3c_fb_win *win = info->par;
>>> - struct s3c_fb *sfb = win->parent;
>>> -
>>> - pm_runtime_put_sync(sfb->dev);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> static struct fb_ops s3c_fb_ops = {
>>> .owner = THIS_MODULE,
>>> - .fb_open = s3c_fb_open,
>>> - .fb_release = s3c_fb_release,
>>> .fb_check_var = s3c_fb_check_var,
>>> .fb_set_par = s3c_fb_set_par,
>>> .fb_blank = s3c_fb_blank,
>>> @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
>>> }
>>>
>>> platform_set_drvdata(pdev, sfb);
>>> - pm_runtime_put_sync(sfb->dev);
>>>
>>> return 0;
>>>
>>> @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
>>> struct s3c_fb *sfb = platform_get_drvdata(pdev);
>>> int win;
>>>
>>> - pm_runtime_get_sync(sfb->dev);
>>> -
>>> for (win = 0; win < S3C_FB_MAX_WIN; win++)
>>> if (sfb->windows[win])
>>> s3c_fb_release_win(sfb, sfb->windows[win]);
>>> @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
>>> return 0;
>>> }
>>>
>>> -#ifdef CONFIG_PM
>>> +#ifdef CONFIG_PM_SLEEP
>>> static int s3c_fb_suspend(struct device *dev)
>>> {
>>> struct platform_device *pdev = to_platform_device(dev);
>>> @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
>>> clk_disable(sfb->lcd_clk);
>>>
>>> clk_disable(sfb->bus_clk);
>>> + pm_runtime_put_sync(sfb->dev);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
>>> struct s3c_fb_win *win;
>>> int win_no;
>>>
>>> + pm_runtime_get_sync(sfb->dev);
>>> clk_enable(sfb->bus_clk);
>>>
>>> if (!sfb->variant.has_clksel)
>>> @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
>>>
>>> return 0;
>>> }
>>> -#else
>>> -#define s3c_fb_suspend NULL
>>> -#define s3c_fb_resume NULL
>>> #endif
>>>
>>> +#ifdef CONFIG_PM_RUNTIME
>>> +static int s3c_fb_runtime_suspend(struct device *dev)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int s3c_fb_runtime_resume(struct device *dev)
>>> +{
>>> + return 0;
>>> +}
>>> +#endif
>>>
>>> #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
>>> #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
>>> @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
>>> };
>>> MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
>>>
>>> -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
>>> +static const struct dev_pm_ops s3c_fb_pm_ops = {
>>> + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
>>> + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
>>> +};
>>>
>>> static struct platform_driver s3c_fb_driver = {
>>> .probe = s3c_fb_probe,
>>> @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
>>> .driver = {
>>> .name = "s3c-fb",
>>> .owner = THIS_MODULE,
>>> - .pm = &s3cfb_pm_ops,
>>> + .pm = &s3c_fb_pm_ops,
>>> },
>>> };
>>>
>
>
^ permalink raw reply
* Re: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Jingoo Han @ 2011-12-19 2:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
Hi Florian,
Florian Tobias Schandinat wrote:
> -----Original Message-----
> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
>
> Hi Jingoo,
>
> On 12/05/2011 02:42 AM, Jingoo Han wrote:
> > Runtime suspend and runtime resume are modified in order to
> > reduce the complexity and improve the usability of runtime pm.
> > After probe function, s3c-fb driver is not suspended until
> > suspend or remove is called.
> >
> > The scheme is changed as follows:
> > runtime_get is only called in probe and resume.
> > runtime_put is only called in remove and suspend.
> > open/close cannot call the runtime_get/put.
> >
> > Also, runtime_susepnd/resume are just called by runtime pm,
> > not doing suspend/resume routine any longer. This is because
> > open/close cannot call the runtime_get/put; the suspend/resume
> > routine in runtime_suspend/resume were previously used when
> > open and close were called.
> >
> > The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
> > s3c_fb_pm_ops in order to use more consistent naming.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>
> the patch itself looks okay to me, but I'm wondering what changes will be
> visible to the user, if any?
> Does the behavior change for example, when no one has called open?
Previously, in order to display framebuffer to LCD panel, user should call
open after booting, because runtime_pm_put is called after prove function.
However, now, after booting, framebuffer can be displayed to LCD panel
without calling open.
Best regards,
Jingoo Han
>
>
> Best regards,
>
> Florian Tobias Schandinat
>
> > ---
> > v2: fix unaligned lines
> >
> > drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
> > 1 files changed, 20 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> > index e84677e..91e629a 100644
> > --- a/drivers/video/s3c-fb.c
> > +++ b/drivers/video/s3c-fb.c
> > @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
> > return ret;
> > }
> >
> > -static int s3c_fb_open(struct fb_info *info, int user)
> > -{
> > - struct s3c_fb_win *win = info->par;
> > - struct s3c_fb *sfb = win->parent;
> > -
> > - pm_runtime_get_sync(sfb->dev);
> > -
> > - return 0;
> > -}
> > -
> > -static int s3c_fb_release(struct fb_info *info, int user)
> > -{
> > - struct s3c_fb_win *win = info->par;
> > - struct s3c_fb *sfb = win->parent;
> > -
> > - pm_runtime_put_sync(sfb->dev);
> > -
> > - return 0;
> > -}
> > -
> > static struct fb_ops s3c_fb_ops = {
> > .owner = THIS_MODULE,
> > - .fb_open = s3c_fb_open,
> > - .fb_release = s3c_fb_release,
> > .fb_check_var = s3c_fb_check_var,
> > .fb_set_par = s3c_fb_set_par,
> > .fb_blank = s3c_fb_blank,
> > @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> > }
> >
> > platform_set_drvdata(pdev, sfb);
> > - pm_runtime_put_sync(sfb->dev);
> >
> > return 0;
> >
> > @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> > struct s3c_fb *sfb = platform_get_drvdata(pdev);
> > int win;
> >
> > - pm_runtime_get_sync(sfb->dev);
> > -
> > for (win = 0; win < S3C_FB_MAX_WIN; win++)
> > if (sfb->windows[win])
> > s3c_fb_release_win(sfb, sfb->windows[win]);
> > @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -#ifdef CONFIG_PM
> > +#ifdef CONFIG_PM_SLEEP
> > static int s3c_fb_suspend(struct device *dev)
> > {
> > struct platform_device *pdev = to_platform_device(dev);
> > @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
> > clk_disable(sfb->lcd_clk);
> >
> > clk_disable(sfb->bus_clk);
> > + pm_runtime_put_sync(sfb->dev);
> > +
> > return 0;
> > }
> >
> > @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
> > struct s3c_fb_win *win;
> > int win_no;
> >
> > + pm_runtime_get_sync(sfb->dev);
> > clk_enable(sfb->bus_clk);
> >
> > if (!sfb->variant.has_clksel)
> > @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
> >
> > return 0;
> > }
> > -#else
> > -#define s3c_fb_suspend NULL
> > -#define s3c_fb_resume NULL
> > #endif
> >
> > +#ifdef CONFIG_PM_RUNTIME
> > +static int s3c_fb_runtime_suspend(struct device *dev)
> > +{
> > + return 0;
> > +}
> > +
> > +static int s3c_fb_runtime_resume(struct device *dev)
> > +{
> > + return 0;
> > +}
> > +#endif
> >
> > #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
> > #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
> > @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> > };
> > MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
> >
> > -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
> > +static const struct dev_pm_ops s3c_fb_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
> > + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
> > +};
> >
> > static struct platform_driver s3c_fb_driver = {
> > .probe = s3c_fb_probe,
> > @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
> > .driver = {
> > .name = "s3c-fb",
> > .owner = THIS_MODULE,
> > - .pm = &s3cfb_pm_ops,
> > + .pm = &s3c_fb_pm_ops,
> > },
> > };
> >
^ permalink raw reply
* [PATCH] MAINTAINERS: add a maintainer for Samsung Framebuffer driver
From: Jingoo Han @ 2011-12-19 2:09 UTC (permalink / raw)
To: linux-fbdev
Add a maintainer for Samsung Framebuffer driver.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Ben Dooks <ben-linux@fluff.org>
---
MAINTAINERS | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 855afd4..a03e288 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5667,6 +5667,12 @@ L: alsa-devel@alsa-project.org (moderated for non-subscribers)
S: Supported
F: sound/soc/samsung
+SAMSUNG FRAMEBUFFER DRIVER
+M: Jingoo Han <jg1.han@samsung.com>
+L: linux-fbdev@vger.kernel.org
+S: Maintained
+F: drivers/video/s3c-fb.c
+
SERIAL DRIVERS
M: Alan Cox <alan@linux.intel.com>
L: linux-serial@vger.kernel.org
--
1.7.1
^ permalink raw reply related
* Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Florian Tobias Schandinat @ 2011-12-19 2:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
Hi Jingoo,
On 12/05/2011 02:42 AM, Jingoo Han wrote:
> Runtime suspend and runtime resume are modified in order to
> reduce the complexity and improve the usability of runtime pm.
> After probe function, s3c-fb driver is not suspended until
> suspend or remove is called.
>
> The scheme is changed as follows:
> runtime_get is only called in probe and resume.
> runtime_put is only called in remove and suspend.
> open/close cannot call the runtime_get/put.
>
> Also, runtime_susepnd/resume are just called by runtime pm,
> not doing suspend/resume routine any longer. This is because
> open/close cannot call the runtime_get/put; the suspend/resume
> routine in runtime_suspend/resume were previously used when
> open and close were called.
>
> The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
> s3c_fb_pm_ops in order to use more consistent naming.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
the patch itself looks okay to me, but I'm wondering what changes will be
visible to the user, if any?
Does the behavior change for example, when no one has called open?
Best regards,
Florian Tobias Schandinat
> ---
> v2: fix unaligned lines
>
> drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
> 1 files changed, 20 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index e84677e..91e629a 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
> return ret;
> }
>
> -static int s3c_fb_open(struct fb_info *info, int user)
> -{
> - struct s3c_fb_win *win = info->par;
> - struct s3c_fb *sfb = win->parent;
> -
> - pm_runtime_get_sync(sfb->dev);
> -
> - return 0;
> -}
> -
> -static int s3c_fb_release(struct fb_info *info, int user)
> -{
> - struct s3c_fb_win *win = info->par;
> - struct s3c_fb *sfb = win->parent;
> -
> - pm_runtime_put_sync(sfb->dev);
> -
> - return 0;
> -}
> -
> static struct fb_ops s3c_fb_ops = {
> .owner = THIS_MODULE,
> - .fb_open = s3c_fb_open,
> - .fb_release = s3c_fb_release,
> .fb_check_var = s3c_fb_check_var,
> .fb_set_par = s3c_fb_set_par,
> .fb_blank = s3c_fb_blank,
> @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> }
>
> platform_set_drvdata(pdev, sfb);
> - pm_runtime_put_sync(sfb->dev);
>
> return 0;
>
> @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> struct s3c_fb *sfb = platform_get_drvdata(pdev);
> int win;
>
> - pm_runtime_get_sync(sfb->dev);
> -
> for (win = 0; win < S3C_FB_MAX_WIN; win++)
> if (sfb->windows[win])
> s3c_fb_release_win(sfb, sfb->windows[win]);
> @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> static int s3c_fb_suspend(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
> clk_disable(sfb->lcd_clk);
>
> clk_disable(sfb->bus_clk);
> + pm_runtime_put_sync(sfb->dev);
> +
> return 0;
> }
>
> @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
> struct s3c_fb_win *win;
> int win_no;
>
> + pm_runtime_get_sync(sfb->dev);
> clk_enable(sfb->bus_clk);
>
> if (!sfb->variant.has_clksel)
> @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
>
> return 0;
> }
> -#else
> -#define s3c_fb_suspend NULL
> -#define s3c_fb_resume NULL
> #endif
>
> +#ifdef CONFIG_PM_RUNTIME
> +static int s3c_fb_runtime_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int s3c_fb_runtime_resume(struct device *dev)
> +{
> + return 0;
> +}
> +#endif
>
> #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
> #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
> @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> };
> MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
>
> -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
> +static const struct dev_pm_ops s3c_fb_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
> + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
> +};
>
> static struct platform_driver s3c_fb_driver = {
> .probe = s3c_fb_probe,
> @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
> .driver = {
> .name = "s3c-fb",
> .owner = THIS_MODULE,
> - .pm = &s3cfb_pm_ops,
> + .pm = &s3c_fb_pm_ops,
> },
> };
>
^ permalink raw reply
* Re: [PATCH 27/57] fbdev: sh_mobile_lcdc: Pass a video mode to the notify callback
From: Laurent Pinchart @ 2011-12-19 1:29 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-28-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Friday 16 December 2011 11:33:55 Guennadi Liakhovetski wrote:
> On Fri, 16 Dec 2011, Laurent Pinchart wrote:
> > On Thursday 15 December 2011 20:01:54 Guennadi Liakhovetski wrote:
> > > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > > Pass pointers to struct fb_videomode and struct fb_monspecs instead
> > > > of struct fb_var_screeninfo to the notify callback.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > >
> > > > drivers/video/sh_mobile_hdmi.c | 59
> > > > +++++++++++++++++--------------------
> > > > drivers/video/sh_mobile_lcdcfb.c
> > > >
> > > > | 40 ++++++++++++++----------- drivers/video/sh_mobile_lcdcfb.h |
> > > >
> > > > 3 +-
> > > > 3 files changed, 51 insertions(+), 51 deletions(-)
> > >
> > > [snip]
> > >
> > > > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > > > b/drivers/video/sh_mobile_lcdcfb.c index 21e5f10..4ec216e 100644
> > > > --- a/drivers/video/sh_mobile_lcdcfb.c
> > > > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > >
> > > [ditto]
> > >
> > > > @@ -433,12 +432,17 @@ static int sh_mobile_lcdc_display_notify(struct
> > > > sh_mobile_lcdc_chan *ch,
> > > >
> > > > }
> > > > break;
> > > >
> > > > - case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
> > > > + case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE: {
> > > > + struct fb_var_screeninfo var;
> > > > +
> > > >
> > > > /* Validate a proposed new mode */
> > > >
> > > > - var->bits_per_pixel = info->var.bits_per_pixel;
> > > > - ret = info->fbops->fb_check_var(var, info);
> > > > + fb_videomode_to_var(&var, mode);
> > > > + var.bits_per_pixel = info->var.bits_per_pixel;
> > > > + var.grayscale = info->var.grayscale;
> > > > + ret = info->fbops->fb_check_var(&var, info);
> > > >
> > > > break;
> > > >
> > > > }
> > > >
> > > > + }
> > >
> > > nitpick - please, realign:-)
> >
> > It's common in driver code not to increase the identation level twice in
> > a case statement if braces are needed. However, those "braced"
> > statements are usually not the last ones in the switch. In this
> > particular case this creates an alignment issue at the end, as your
> > correctly pointed out. However, I'd like to avoid increasing the
> > indentation of the whole block. Increasing the indentation of the
> > closing brace only also causes an alignment issue. I can add a default
> > case that returns an error to fix this, but that's adding code to fix a
> > cosmetic problem :-) What's your preference.
>
> Personally, I don't like just using braces without a related statement
> like "if," "do," etc. In such "case" cases I usually avoid adding own
> braces and just put any declarations, that are needed there in the
> enclosing block, e.g., the function.
Using braces with case statements is a common practice in the kernel, without
adding an extra indentation level.
In this particular case I can move the variable declaration at the top of the
function, as the generated code on ARM (at least with gcc ('cs2009q3-67-hard-
sb4') 4.4.1) doesn't differ.
> If you really don't want either, I would probably prefer a style like
>
> switch (x) {
> case X:
> {
> int y;
> my_statement(y);
> break;
> }
> }
>
> You could also do
>
> switch (x) {
> case X:
> do {
> int y;
> my_statement(y);
> break;
> } while (0);
> }
>
> So, choose your poison:-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to
From: Guennadi Liakhovetski @ 2011-12-19 0:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-13-git-send-email-laurent.pinchart@ideasonboard.com>
On Fri, 16 Dec 2011, Laurent Pinchart wrote:
> Hi Guennadi,
>
> On Thursday 15 December 2011 15:01:27 Guennadi Liakhovetski wrote:
> > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > Make sure the transmitter devices get registered before the associated
> > > LCDC devices.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> [snip]
>
> > > diff --git a/arch/arm/mach-shmobile/board-ap4evb.c
> > > b/arch/arm/mach-shmobile/board-ap4evb.c index dca4860..e0a6b3d 100644
> > > --- a/arch/arm/mach-shmobile/board-ap4evb.c
> > > +++ b/arch/arm/mach-shmobile/board-ap4evb.c
>
> [snip]
>
> > > @@ -627,6 +553,86 @@ static struct platform_device *qhd_devices[]
> > > __initdata = {
> > >
> > > };
> > > #endif /* CONFIG_AP4EVB_QHD */
> > >
> > > +/* LCDC0 */
> > > +static const struct fb_videomode ap4evb_lcdc_modes[] = {
> >
> > Hm, I must be missing something. I thought you were moving all the structs
> > around to make them available for reference _without_ forward
> > declarations. But you _do_ end up using a forward declaration anyway. Here
> > and for HDMI below too. So, what's the point? Why not just forward declare
> > that one struct, that's needed and leave the rest where it currently is
> > in the file?
>
> As we have circular dependencies forward declaration can't be avoided. I found
> it cleaner to have devices data in registration order in the board file, but I
> can remove this if you prefer.
As a _general_ guidline I find smaller patches more pleasant than
bigger;-) but if you really see an important advantage in moving these
structs around - I can live with that too:-)
> > > + {
> > > +#ifdef CONFIG_AP4EVB_QHD
> > > + .name = "R63302(QHD)",
> > > + .xres = 544,
> > > + .yres = 961,
> > > + .left_margin = 72,
> > > + .right_margin = 600,
> > > + .hsync_len = 16,
> > > + .upper_margin = 8,
> > > + .lower_margin = 8,
> > > + .vsync_len = 2,
> > > + .sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> > > +#else
> > > + .name = "WVGA Panel",
> > > + .xres = 800,
> > > + .yres = 480,
> > > + .left_margin = 220,
> > > + .right_margin = 110,
> > > + .hsync_len = 70,
> > > + .upper_margin = 20,
> > > + .lower_margin = 5,
> > > + .vsync_len = 5,
> > > + .sync = 0,
> > > +#endif
> > > + },
> > > +};
>
> [snip]
>
> > > diff --git a/arch/arm/mach-shmobile/board-mackerel.c
> > > b/arch/arm/mach-shmobile/board-mackerel.c index 4ed0138..ccdd9fc 100644
> > > --- a/arch/arm/mach-shmobile/board-mackerel.c
> > > +++ b/arch/arm/mach-shmobile/board-mackerel.c
>
> [snip]
>
> > > @@ -1313,8 +1317,8 @@ static struct platform_device *mackerel_devices[]
> > > __initdata = {
> > > &sh_mmcif_device,
> > > &ceu_device,
> > > &mackerel_camera,
> > > - &hdmi_lcdc_device,
> > > &hdmi_device,
> > > + &hdmi_lcdc_device,
> > > &meram_device,
> > > };
> >
> > Sorry, could you elaborate, why this is needed? If this is really
> > important, then I'd prefer to test this before committing. If OTOH this is
> > unimportant, and my assumption, that normally it's the platform driver
> > registration order, that's important, not device registration order, is
> > correct, then you don't have to swap the order?
>
> If both the transmitter and LCDC drivers are built-in, the device probe order
> is the device registration order. As we need the transmitter to be probed
> first, it needs to be registered first.
Even there I'm not quite sure about - I do remember having to fiddle with
the Makefiles to change the probe order, which followed the linkage order.
Could be, that this has changed since I last checked, but I'd really
double-check this.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 24/57] fbdev: sh_mobile_lcdc: Return display connection state in display_on
From: Laurent Pinchart @ 2011-12-19 0:21 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-25-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Thursday 15 December 2011 18:11:03 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > Return true if the display is connected and false otherwise. Set the fb
> > info state to FBINFO_STATE_SUSPENDED in the sh_mobile_lcdc driver when
> > the display is not connected.
>
> Hmm... I'm not sure I like functions, that return either a negative error,
> or _several_ non-negative success values. How about returning -ENODEV?
That would be returning a negative error on success. Is that better ? :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v4] Resurrect Intel740 driver: i740fb
From: Andrew Morton @ 2011-12-17 1:12 UTC (permalink / raw)
To: Ondrej Zary
Cc: linux-fbdev, Florian Tobias Schandinat, Paul Mundt,
Kernel development list
In-Reply-To: <201112080024.24383.linux@rainbow-software.org>
On Thu, 8 Dec 2011 00:24:18 +0100
Ondrej Zary <linux@rainbow-software.org> wrote:
> This is a resurrection of an old (like 2.4.19) out-of-tree driver for
> Intel740 graphics cards and modify it for recent kernels. The old driver is
> located at: http://sourceforge.net/projects/i740fbdev/files/
>
> This is a new driver based on skeletonfb, using most of the low level HW code
> from the old driver. The DDC code is completely new.
>
> The driver was tested on two 8MB cards: Protac AG240D and Diamond Stealth II
> G460.
>
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
The original driver appears to have been written by Andrey Ulanov. It
would be nice to mention that in the changelog and it is desirable that
you seek his Signed-off-by:, please.
> Changes in v4:
> - shortened many lines to fit 80-char limit
> - removed useless inb_p and outb_p functions
> - converted DDC (and some other) code to i740outreg_mask
> - removed useless wm initialization in i740_calc_fifo()
> - ALIGN macro is used instead of manual alignment
> - removed inactive code
> - simplified i740_setcolreg()
> - info->var used for panning
> - fixed out-of-video-memory checking
It still generates a large number of checkpatch errors. If you've
reviewed those and made the decision to ignore them then OK(ish).
Otherwise, please check.
>
> ...
>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/fb.h>
> +#include <linux/init.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-algo-bit.h>
Should there have been a dependency on i2c in the Kconfig entry? If
not, has the driver been tested with is2 disabled?
> +#include <linux/console.h>
> +#include <video/vga.h>
> +
>
> ...
>
> +#define I740_RFREQ (1e6)
> +#define TARGET_MAX_N 30
> +
> +#define I740_FFIX 8
> +#define I740_REF_FREQ (u32) (66.66666666667 * (1 << I740_FFIX) + 0.5)
> +#define I740_MAX_VCO_FREQ (u32)(450.00000000000 * (1 << I740_FFIX) + 0.5)
There should be no floating point in the kernel, normally. Given the
casts, these will obviously not get any further than the compiler. But
if these could be redone to avoid the float, that would be nice.
> +#define I740_CALC_VCLKfix(m, n, p, d) ((((((m) * I740_REF_FREQ * (4 << ((d) << 1)))) / (n)) + ((1 << (p)) / 2)) / (1 << (p)))
This macro should be dragged out and shot. And it will cause bugs if
invoked using an expression with side-effects. There's no need to do
this to ourselves - please turn it into a nice, lower-case-named C
function with proper identifiers, code comments, etc.
> +static void i740_calc_vclk(u32 freq_hz, struct i740fb_par *par)
> +{
> + u32 freq = freq_hz / (u32)(1e6 / I740_RFREQ);
> + const u32 err_max > + freq / (u32)(I740_RFREQ / 0.005 / (1 << I740_FFIX) + 0.5);
> + const u32 err_target > + freq / (u32)(I740_RFREQ / 0.001 / (1 << I740_FFIX) + 0.5);
> + u32 err_best = (u32)(512.0 * (1 << I740_FFIX));
> + u32 f_err, f_vco;
> + int m_best = 0, n_best = 0, p_best = 0, d_best = 0;
> + int m, n, i;
> +
> + /* find log2(MAX_VCO_FREQ/f_target) */
> + for (i = 0; i < 16; i++)
> + if ((I740_MAX_VCO_FREQ) / (1 << i) < freq / (u32)(I740_RFREQ / (1 << I740_FFIX)))
Use the existing ilog2()?
> + break;
> + i--;
> + p_best = i;
> +
> + d_best = 0;
> + f_vco = (freq * (1 << p_best)) / (u32)(I740_RFREQ / (1 << I740_FFIX));
> + freq = freq / (u32)(I740_RFREQ / (1 << I740_FFIX));
> +
> + n = 2;
> + do {
> + n++;
> + m = ((f_vco * n) / I740_REF_FREQ + 2) / 4;
> +
> + if (m < 3)
> + m = 3;
> +
> + {
> + u32 f_out = I740_CALC_VCLKfix(m, n, p_best, d_best);
> +
> + f_err = (freq - f_out);
> +
> + if (abs(f_err) < err_max) {
> + m_best = m;
> + n_best = n;
> + err_best = f_err;
> + }
> + }
> + } while ((abs(f_err) >= err_target) &&
> + ((n <= TARGET_MAX_N) || (abs(err_best) > err_max)));
> +
> + if (abs(f_err) < err_target) {
> + m_best = m;
> + n_best = n;
> + }
> +
> + par->video_clk2_m = (m_best-2) & 0xFF;
> + par->video_clk2_n = (n_best-2) & 0xFF;
> + par->video_clk2_mn_msbs = ((((n_best-2) >> 4) & VCO_N_MSBS)
> + | (((m_best-2) >> 8) & VCO_M_MSBS));
> + par->video_clk2_div_sel > + ((p_best << 4) | (d_best ? 4 : 0) | REF_DIV_1);
> +}
> +
>
> ...
>
> +#ifndef MODULE
> +static int __init i740fb_setup(char *options)
> +{
> + char *opt;
> +
> + if (!options || !*options)
> + return 0;
> +
> + while ((opt = strsep(&options, ",")) != NULL) {
> + if (!*opt)
> + continue;
> +#ifdef CONFIG_MTRR
> + else if (!strncmp(opt, "mtrr:", 5))
> + mtrr = simple_strtoul(opt + 5, NULL, 0);
> +#endif
> + else
> + mode_option = opt;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +int __init i740fb_init(void)
> +{
> +#ifndef MODULE
> + char *option = NULL;
> +
> + if (fb_get_options("i740fb", &option))
> + return -ENODEV;
> + i740fb_setup(option);
> +#endif
> +
> + return pci_register_driver(&i740fb_driver);
> +}
hm, the "ifdef MODULE" stuff looks old-fashioned, but quite a few fbdev
drivers appear to be doing it.
>
> ...
>
Should we also be adding a documentation file for thei driver? Tell
people what the various module options do?
^ permalink raw reply
* Re: "ls > /dev/fb0" generates weird ioctls if panning is set
From: Anatolij Gustschin @ 2011-12-16 23:03 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4EEBBCEA.9020104@freescale.com>
On Fri, 16 Dec 2011 16:31:22 -0600
Timur Tabi <timur@freescale.com> wrote:
> Timur Tabi wrote:
>
> > graphics fb0: unknown ioctl command (0x402C7413)
> > graphics fb0: dir=2 type='t' (74) nr\x19 sizeD
> > graphics fb0: unknown ioctl command (0x40087468)
> > graphics fb0: dir=2 type='t' (74) nr\x104 size=8
> > graphics fb0: unknown ioctl command (0x402C7413)
> > graphics fb0: dir=2 type='t' (74) nr\x19 sizeD
>
> I figured it out. These ioctls are:
>
> #define TCGETS _IOR('t', 19, struct termios)
> #define TIOCGWINSZ _IOR('t', 104, struct winsize)
>
> I'm guessing that since I redirect stdout to /dev/fb0, Linux is
> treating /dev/fb0 as a terminal, and therefore it's receiving
> terminal ioctls.
>
> Is this something that fbdev should be supporting, or am I wrong
> to use /dev/fb0 as a terminal?
No. Do not redirect to /dev/fb0, use /dev/tty1 instead.
Anatolij
^ permalink raw reply
* Re: [PATCH v4 3/5] powerpc/mpc5121: shared DIU framebuffer support
From: Timur Tabi @ 2011-12-16 23:00 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1279893639-24333-4-git-send-email-agust@denx.de>
Anatolij Gustschin wrote:
> Will DIU splash screen functionality be preserved?
That's the idea. The root cause is that the drive initializes the DIU during the probe, which is too early. It shouldn't touch the hardware until the open. At that point, Linux (X, console, whatever) wants to draw something.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH v4 3/5] powerpc/mpc5121: shared DIU framebuffer support
From: Anatolij Gustschin @ 2011-12-16 22:59 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1279893639-24333-4-git-send-email-agust@denx.de>
On Fri, 16 Dec 2011 12:24:49 -0600
Timur Tabi <timur@freescale.com> wrote:
> Anatolij Gustschin wrote:
>
> > We cannot guarantee that the DIU is not in these modes. Even
> > if U-Boot didn't set these modes there is still a possibility
> > that such mode is configured. E.g. I've seen U-Boot binary
> > standalone applications for other display controllers initializing
> > the display controller.
>
> True, but modes 2 and 3 don't make any sense. 2 is just a color bar,
> and 3 writes back to memory (which means you need to have a reserved
> DMA buffer otherwise you'll crash). Have you really seen Linux boot
> with the DIU in either of these modes?
No, but it doesn't mean that it is not possible.
> > But you are right. With this snippet, if the DIU is already
> > disabled, there will be not needed mode register access. So
> > the code should better look like:
> >
> > diu_mode = in_be32(&dr.diu_reg->diu_mode);
> > if (diu_mode && diu_mode != MFB_MODE1)
> > out_be32(&dr.diu_reg->diu_mode, 0);
>
> Ok. I'm planning on cleaning up the driver so that it does not
> initialize the DIU until the .open call, and once that's done,
> I should be able to remove most of the code from your patch.
Will DIU splash screen functionality be preserved?
Anatolij
^ permalink raw reply
* Re: "ls > /dev/fb0" generates weird ioctls if panning is set
From: Timur Tabi @ 2011-12-16 22:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4EEBBCEA.9020104@freescale.com>
Timur Tabi wrote:
> graphics fb0: unknown ioctl command (0x402C7413)
> graphics fb0: dir=2 type='t' (74) nr\x19 sizeD
> graphics fb0: unknown ioctl command (0x40087468)
> graphics fb0: dir=2 type='t' (74) nr\x104 size=8
> graphics fb0: unknown ioctl command (0x402C7413)
> graphics fb0: dir=2 type='t' (74) nr\x19 sizeD
I figured it out. These ioctls are:
#define TCGETS _IOR('t', 19, struct termios)
#define TIOCGWINSZ _IOR('t', 104, struct winsize)
I'm guessing that since I redirect stdout to /dev/fb0, Linux is treating /dev/fb0 as a terminal, and therefore it's receiving terminal ioctls.
Is this something that fbdev should be supporting, or am I wrong to use /dev/fb0 as a terminal?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* "ls > /dev/fb0" generates weird ioctls if panning is set
From: Timur Tabi @ 2011-12-16 21:49 UTC (permalink / raw)
To: linux-fbdev
If I do this:
echo "0,100" > /sys/devices/soc.0/fffe10000.display/graphics/fb0/pan
then whenever I do this:
ls > /dev/fb0
my driver receives the following three ioctls:
graphics fb0: unknown ioctl command (0x402C7413)
graphics fb0: dir=2 type='t' (74) nr\x19 sizeD
graphics fb0: unknown ioctl command (0x40087468)
graphics fb0: dir=2 type='t' (74) nr\x104 size=8
graphics fb0: unknown ioctl command (0x402C7413)
graphics fb0: dir=2 type='t' (74) nr\x19 sizeD
This is on PowerPC, where dir=2 means read.
The "echo" and "cat" commands don't generate these ioctls, so there's something special about the "ls" command. Does anyone know what's going on? ioctl-number.txt lists these ioctls for type 't':
't' 00-7F linux/if_ppp.h
't' 80-8F linux/isdn_ppp.h
't' 90 linux/toshiba.h
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Should I use FBINFO_VIRTFB?
From: Geert Uytterhoeven @ 2011-12-16 20:16 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4EEA5DA7.3010406@freescale.com>
On Fri, Dec 16, 2011 at 19:18, Timur Tabi <timur@freescale.com> wrote:
> Geert Uytterhoeven wrote:
>> This depends on which of these two is fastest for scrolling the console:
>> - panning the virtual screen and redrawing the missing part: set
>> FBINFO_PARTIAL_PAN_OK
>> - copying or redrawing the screen: don't set FBINFO_PARTIAL_PAN_OK
>
> I'm a little confused about panning support in my driver (which I didn't write -- I'm just cleaning it up). There is .fb_pan_display function, but it only gets called early in the boot process, and only with xoffset=0 and yoffset=0. After that, it never seems to get called again. Under what circumstances is panning really used?
Is yres_virtual larger than yres? Is the font size a multiple of ypanstep?
If not, panning is disabled.
>>> I have the same problem with FBINFO_READS_FAST.
>>
>> You should set this flag if reading from frame buffer memory is a fast
>> operation.
>> On many graphics devices, reading from frame buffer memory is much slower
>> than writing. As you use system RAM, you probably want to set it.
>
> I see a lot of drivers that set FBINFO_VIRTFB but don't set FBINFO_READS_FAST. Why?
>
>> If this flag is set, scrolling is implemented by copying memory around.
>> If not set, scrolling is implemented by redrawing the whole screen.
>>
>> A simple way to find the optimal settings of both flags (all 4 combinations) is
>> running "clear; time cat big_text_file" and comparing the timing results
>
> That's the funny thing -- I've been trying various combinations of FBINFO_VIRTFB, FBINFO_PARTIAL_PAN_OK, and FBINFO_READS_FAST, and I always get the same timing result. It seems to have no affect on performance.
If panning is disabled, FBINFO_PARTIAL_PAN_OK doesn't do anything.
For FBINFO_READS_FAST, I can imagine that with a fast CPU and a good
writeback cache, redrawing the screen is as fast as copying data around.
It just means your hardware is well balanced ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v4 3/5] powerpc/mpc5121: shared DIU framebuffer support
From: Timur Tabi @ 2011-12-16 18:24 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1279893639-24333-4-git-send-email-agust@denx.de>
Anatolij Gustschin wrote:
> We cannot guarantee that the DIU is not in these modes. Even
> if U-Boot didn't set these modes there is still a possibility
> that such mode is configured. E.g. I've seen U-Boot binary
> standalone applications for other display controllers initializing
> the display controller.
True, but modes 2 and 3 don't make any sense. 2 is just a color bar, and 3 writes back to memory (which means you need to have a reserved DMA buffer otherwise you'll crash). Have you really seen Linux boot with the DIU in either of these modes?
> But you are right. With this snippet, if the DIU is already
> disabled, there will be not needed mode register access. So
> the code should better look like:
>
> diu_mode = in_be32(&dr.diu_reg->diu_mode);
> if (diu_mode && diu_mode != MFB_MODE1)
> out_be32(&dr.diu_reg->diu_mode, 0);
Ok. I'm planning on cleaning up the driver so that it does not initialize the DIU until the .open call, and once that's done, I should be able to remove most of the code from your patch.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Should I use FBINFO_VIRTFB?
From: Timur Tabi @ 2011-12-16 18:18 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4EEA5DA7.3010406@freescale.com>
Geert Uytterhoeven wrote:
> This depends on which of these two is fastest for scrolling the console:
> - panning the virtual screen and redrawing the missing part: set
> FBINFO_PARTIAL_PAN_OK
> - copying or redrawing the screen: don't set FBINFO_PARTIAL_PAN_OK
I'm a little confused about panning support in my driver (which I didn't write -- I'm just cleaning it up). There is .fb_pan_display function, but it only gets called early in the boot process, and only with xoffset=0 and yoffset=0. After that, it never seems to get called again. Under what circumstances is panning really used?
>
>> I have the same problem with FBINFO_READS_FAST.
>
> You should set this flag if reading from frame buffer memory is a fast
> operation.
> On many graphics devices, reading from frame buffer memory is much slower
> than writing. As you use system RAM, you probably want to set it.
I see a lot of drivers that set FBINFO_VIRTFB but don't set FBINFO_READS_FAST. Why?
> If this flag is set, scrolling is implemented by copying memory around.
> If not set, scrolling is implemented by redrawing the whole screen.
>
> A simple way to find the optimal settings of both flags (all 4 combinations) is
> running "clear; time cat big_text_file" and comparing the timing results
That's the funny thing -- I've been trying various combinations of FBINFO_VIRTFB, FBINFO_PARTIAL_PAN_OK, and FBINFO_READS_FAST, and I always get the same timing result. It seems to have no affect on performance.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Should I use FBINFO_VIRTFB?
From: Geert Uytterhoeven @ 2011-12-16 11:53 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4EEA5DA7.3010406@freescale.com>
On Thu, Dec 15, 2011 at 23:10, Timur Tabi <timur@freescale.com> wrote:
> Konrad Rzeszutek Wilk wrote:
>>> > #define FBINFO_PARTIAL_PAN_OK 0x0040 /* otw use pan only for double-buffering */
>>> > #define FBINFO_READS_FAST 0x0080 /* soft-copy faster than rendering */
>
>> Not really. They have a different function, but you are better of looking in the code
>> to see how they are used.
> I can't parse this, and I can't figure out if my driver is better off with or without FBINFO_PARTIAL_PAN_OK.
This depends on which of these two is fastest for scrolling the console:
- panning the virtual screen and redrawing the missing part: set
FBINFO_PARTIAL_PAN_OK
- copying or redrawing the screen: don't set FBINFO_PARTIAL_PAN_OK
> I have the same problem with FBINFO_READS_FAST.
You should set this flag if reading from frame buffer memory is a fast
operation.
On many graphics devices, reading from frame buffer memory is much slower
than writing. As you use system RAM, you probably want to set it.
If this flag is set, scrolling is implemented by copying memory around.
If not set, scrolling is implemented by redrawing the whole screen.
A simple way to find the optimal settings of both flags (all 4 combinations) is
running "clear; time cat big_text_file" and comparing the timing results
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 27/57] fbdev: sh_mobile_lcdc: Pass a video mode to the
From: Guennadi Liakhovetski @ 2011-12-16 10:33 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-28-git-send-email-laurent.pinchart@ideasonboard.com>
On Fri, 16 Dec 2011, Laurent Pinchart wrote:
> Hi Guennadi,
>
> On Thursday 15 December 2011 20:01:54 Guennadi Liakhovetski wrote:
> > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > Pass pointers to struct fb_videomode and struct fb_monspecs instead of
> > > struct fb_var_screeninfo to the notify callback.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >
> > > drivers/video/sh_mobile_hdmi.c | 59
> > > +++++++++++++++++-------------------- drivers/video/sh_mobile_lcdcfb.c
> > > | 40 ++++++++++++++----------- drivers/video/sh_mobile_lcdcfb.h |
> > > 3 +-
> > > 3 files changed, 51 insertions(+), 51 deletions(-)
> >
> > [snip]
> >
> > > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > > b/drivers/video/sh_mobile_lcdcfb.c index 21e5f10..4ec216e 100644
> > > --- a/drivers/video/sh_mobile_lcdcfb.c
> > > +++ b/drivers/video/sh_mobile_lcdcfb.c
> >
> > [ditto]
> >
> > > @@ -433,12 +432,17 @@ static int sh_mobile_lcdc_display_notify(struct
> > > sh_mobile_lcdc_chan *ch,
> > >
> > > }
> > > break;
> > >
> > > - case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
> > > + case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE: {
> > > + struct fb_var_screeninfo var;
> > > +
> > >
> > > /* Validate a proposed new mode */
> > >
> > > - var->bits_per_pixel = info->var.bits_per_pixel;
> > > - ret = info->fbops->fb_check_var(var, info);
> > > + fb_videomode_to_var(&var, mode);
> > > + var.bits_per_pixel = info->var.bits_per_pixel;
> > > + var.grayscale = info->var.grayscale;
> > > + ret = info->fbops->fb_check_var(&var, info);
> > >
> > > break;
> > >
> > > }
> > >
> > > + }
> >
> > nitpick - please, realign:-)
>
> It's common in driver code not to increase the identation level twice in a
> case statement if braces are needed. However, those "braced" statements are
> usually not the last ones in the switch. In this particular case this creates
> an alignment issue at the end, as your correctly pointed out. However, I'd
> like to avoid increasing the indentation of the whole block. Increasing the
> indentation of the closing brace only also causes an alignment issue. I can
> add a default case that returns an error to fix this, but that's adding code
> to fix a cosmetic problem :-) What's your preference.
Personally, I don't like just using braces without a related statement
like "if," "do," etc. In such "case" cases I usually avoid adding own
braces and just put any declarations, that are needed there in the
enclosing block, e.g., the function. If you really don't want either, I
would probably prefer a style like
switch (x) {
case X:
{
int y;
my_statement(y);
break;
}
}
You could also do
switch (x) {
case X:
do {
int y;
my_statement(y);
break;
} while (0);
}
So, choose your poison:-)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 25/57] sh_mobile_lcdc: Add display notify callback to sh_mobile_lcdc_chan
From: Laurent Pinchart @ 2011-12-16 10:22 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-26-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Friday 16 December 2011 10:40:48 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > The callback implements 3 notification events:
> >
> > - SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT notifies the LCDC that the
> > display has been connected
> >
> > - SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT notifies the LCDC that the
> > display has been disconnected
> >
> > - SH_MOBILE_LCDC_EVENT_DISPLAY_MODE notifies that LCDC that a display
> > mode has been detected
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > b/drivers/video/sh_mobile_lcdcfb.c index 4b03aa5..21e5f10 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > @@ -363,6 +363,86 @@ static void sh_mobile_lcdc_display_off(struct
[snip]
> > +static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
> > + enum sh_mobile_lcdc_entity_event event,
> > + struct fb_var_screeninfo *var)
> > +{
> > + struct fb_info *info = ch->info;
> > + int ret = 0;
> > +
> > + switch (event) {
> > + case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT:
> > + /* HDMI plug in */
> > + if (lock_fb_info(info)) {
> > + console_lock();
> > +
> > + if (!sh_mobile_lcdc_must_reconfigure(ch, var) &&
> > + info->state = FBINFO_STATE_RUNNING) {
> > + /* First activation with the default monitor.
> > + * Just turn on, if we run a resume here, the
> > + * logo disappears.
> > + */
> > + info->var.width = var->width;
> > + info->var.height = var->height;
> > + sh_mobile_lcdc_display_on(ch);
> > + } else {
> > + /* New monitor or have to wake up */
> > + fb_set_suspend(info, 0);
> > + }
> > +
> > + console_unlock();
> > + unlock_fb_info(info);
> > + }
> > + break;
> > +
> > + case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT:
> > + /* HDMI disconnect */
> > + if (lock_fb_info(info)) {
> > + console_lock();
> > + fb_set_suspend(info, 1);
> > + console_unlock();
> > + unlock_fb_info(info);
> > + }
> > + break;
> > +
> > + case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
> > + /* Validate a proposed new mode */
> > + var->bits_per_pixel = info->var.bits_per_pixel;
> > + ret = info->fbops->fb_check_var(var, info);
>
> You can call sh_mobile_lcdc_check_var() directly here too, right?
This was a way to avoid a forward declaration. You would prefer a forward
declaration, right ? :-) I think you would be right.
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
--
Regards,
Laurent Pinchart
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox