* Re: [V2 1/7] video: mmp: rb swap setting update for new LCD driver
From: Daniel Drake @ 2013-06-21 16:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1370879533-11017-1-git-send-email-jtzhou@marvell.com>
On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
> From: Guoqing Li <ligq@marvell.com>
>
> We could set rb swap in two modules: DMA controler and interface
> output after blending.
> This patch move the panel rbswap requirement setting in later module.
link_config originates from the platform's path config. link_config is
undocumented and this patch also changes its meaning.
Previously, bit 0 triggered rbswap, and this behaviour is relied upon
by arch/arm/mach-mmp/ttc_dkb.c
Now bits 27:24 of link_config are used to enable or disable rbswap,
and link_config bit 0 is ignored. According to the specs for the panel
path register, valid values for these new bits are 0 (no swap) and 1
(swap). ttc_dkb has not been updated in this patch series for this new
behaviour.
I don't understand why rbswap is set to 1 in fmt_to_reg for certain
RGB formats. The patch description suggests that the rb swapping can
either be set in fmt_to_reg context (to be written into DMA
controller) *or* in the interface output. If we are now relying on the
interface output control to do RB swapping when appropriate, why are
there still cases when we ask the DMA controller to do the same thing?
I also do not fully understand the requirement for RB swapping. I do
understand that it changes pixel format from RGB to BGR. What is the
point? I can imagine that some panels may require such a pixel format,
however in that case, this configuration should be part of the panel
configuration, not part of the path configuration as it currently is.
Daniel
^ permalink raw reply
* Re: [V2 2/7] video: mmp: optimize some register setting code
From: Daniel Drake @ 2013-06-21 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1370879543-11152-1-git-send-email-jtzhou@marvell.com>
On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
> From: Guoqing Li <ligq@marvell.com>
>
> There are dumplicate code of the smooth setting based on different
> path, optimized the routine and use readl_relaxed instead.
>
> Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
> Signed-off-by: Jing Xiang <jxiang@marvell.com>
> Signed-off-by: Guoqing Li <ligq@marvell.com>
Reviewed-by: Daniel Drake <dsd@laptop.org>
^ permalink raw reply
* Re: [V2 3/7] video: mmp: fix graphics/video layer enable/mask swap issue
From: Daniel Drake @ 2013-06-21 17:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1370879552-11290-1-git-send-email-jtzhou@marvell.com>
On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
> From: Jing Xiang <jxiang@marvell.com>
>
> There is bug when switch dma of graphic layer and video layer, it
> configured opposite bit, fix it.
So, overlays can be either graphics or video. overlay_is_vid() tells
you which type.
overlay_is_vid() makes its decision based on dmafetch_id which is
undocumented and I don't understand it (and in another mail you said
this is legacy configuration that is going to be removed).
Ignoring my lack of understanding of the background: the patch here
seems to make the code more correct. It will modify the path's
underlying control register to enable graphics or video transfer
depending on the overlay type. Turning a graphics overlay dmafetch ON
will not stomp on the bits set by any video overlay dmafetch on the
same path. Previously the logic was reversed.
This codepath will not work correctly if there will ever be more than
one overlay of the same type on the same path. For example, if we have
two graphics overlays;
1. dmafetch_onoff(overlay1, on) - graphics transfer is now enabled in
the hardware
2. dmafetch_onoff(overlay2, on) - graphics transfer was already
enabled, no change
3. dmafetch_onoff(overlay2, off) - graphics transfer is now disabled
in the hardware
At this point overlay1 is still active in theory, but the driver has
disabled graphics transfer at the hardware level.
Daniel
^ permalink raw reply
* Re: [V2 4/7] video: mmp: fix memcpy wrong size for mmp_addr issue
From: Daniel Drake @ 2013-06-21 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1370879562-11342-1-git-send-email-jtzhou@marvell.com>
On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
> From: Jing Xiang <jxiang@marvell.com>
>
> Memcpy used wrong struct of mmp_win, fix it.
>
> Signed-off-by: Jing Xiang <jxiang@marvell.com>
> Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
Reviewed-by: Daniel Drake <dsd@laptop.org>
^ permalink raw reply
* Re: [V2 5/7] video: mmp: add pitch info in mmp_win structure
From: Daniel Drake @ 2013-06-21 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1370879574-11397-1-git-send-email-jtzhou@marvell.com>
On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
> From: Jing Xiang <jxiang@marvell.com>
>
> Add pitch length info of graphics/video layer for mmp_win, if it is
> YUV format of video layer, u/v pitch will non-zero.
>
> Signed-off-by: Jing Xiang <jxiang@marvell.com>
> Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
> ---
> include/video/mmp_disp.h | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
> index b9dd1fb..462e3bd 100644
> --- a/include/video/mmp_disp.h
> +++ b/include/video/mmp_disp.h
> @@ -91,6 +91,11 @@ struct mmp_win {
> u16 up_crop;
> u16 bottom_crop;
> int pix_fmt;
> + /*
> + * pitch[0]: graphics/video layer line length or y pitch
> + * pitch[1]/pitch[2]: video u/v pitch if non-zero
> + */
> + u32 pitch[3];
> };
Thanks for adding a comment here, but the meaning of this field is
still not clear to me.
In what case is pitch[0] line length, and in which case does it refer
to y pitch?
pitch[1] and pitch[2] refer to u/v pitch respectively, if their own
values are non-zero? (or if not, what value does the "if non-zero"
comment refer to?)
I would recommend rolling this patch into the patch that actually
makes use of this new field.
Daniel
^ permalink raw reply
* Re: [V2 6/7] video: mmp: calculate pitch value when fb set win
From: Daniel Drake @ 2013-06-21 17:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1370879585-11452-1-git-send-email-jtzhou@marvell.com>
On Mon, Jun 10, 2013 at 9:53 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
> From: Jing Xiang <jxiang@marvell.com>
>
> Add new func mmpfb_set_win to make code clean, it will calculate pitch
> value when fb set win in mmpfb_set_win.
It looks like a good idea to create a function for setting window
parameters from fb info, but this patch also starts to write into the
pitch[] fields which are not used anywhere. You should rework these
patches so that the pitch field is introduced, set, and consumed all
in the same patch.
The commit message for the patch that added pitch[] seems to suggest
that pitch[1] and pitch[2] are used for YUV formats. But in the code
posted here, they will not be used for PIXFMT_YUYV, PIXFMT_UYVY, etc.
Thanks,
Daniel
^ permalink raw reply
* Re: [V2 7/7] video: mmp: add video layer set win/addr operations support
From: Daniel Drake @ 2013-06-21 17:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1370879593-11557-1-git-send-email-jtzhou@marvell.com>
On Mon, Jun 10, 2013 at 9:53 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
> From: Jing Xiang <jxiang@marvell.com>
>
> Since mmp display controller support graphic/video layer together, so
> add win/addr setting operations support when set win parameters.
> Signed-off-by: Jing Xiang <jxiang@marvell.com>
> Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
Still confused by the implementation of overlay_is_vid(), but on the
basis that overlays can be either video or graphics, the code changes
here seem to fit with the device specs.
Reviewed-by: Daniel Drake <dsd@laptop.org>
^ permalink raw reply
* [PATCH 1/2] fb: backlight: HX8357: Make IM pins optionnal
From: Alexandre Belloni @ 2013-06-21 18:27 UTC (permalink / raw)
To: linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD
Cc: jimwall, brian, Maxime Ripard, linux-kernel, Richard Purdie,
Florian Tobias Schandinat, Alexandre Belloni
From: Maxime Ripard <maxime.ripard@free-electrons.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 53 +++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index a0482b5..69f5672 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -76,6 +76,7 @@ struct hx8357_data {
unsigned reset;
struct spi_device *spi;
int state;
+ u8 use_im_pins;
};
static u8 hx8357_seq_power[] = {
@@ -250,9 +251,11 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
* Set the interface selection pins to SPI mode, with three
* wires
*/
- gpio_set_value_cansleep(lcd->im_pins[0], 1);
- gpio_set_value_cansleep(lcd->im_pins[1], 0);
- gpio_set_value_cansleep(lcd->im_pins[2], 1);
+ if (lcd->use_im_pins) {
+ gpio_set_value_cansleep(lcd->im_pins[0], 1);
+ gpio_set_value_cansleep(lcd->im_pins[1], 0);
+ gpio_set_value_cansleep(lcd->im_pins[2], 1);
+ }
/* Reset the screen */
gpio_set_value(lcd->reset, 1);
@@ -424,26 +427,32 @@ static int hx8357_probe(struct spi_device *spi)
return -EINVAL;
}
- for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
- lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
- "im-gpios", i);
- if (lcd->im_pins[i] = -EPROBE_DEFER) {
- dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
- return -EPROBE_DEFER;
- }
- if (!gpio_is_valid(lcd->im_pins[i])) {
- dev_err(&spi->dev, "Missing dt property: im-gpios\n");
- return -EINVAL;
+ if (of_find_property(spi->dev.of_node, "im-gpios", NULL)) {
+ lcd->use_im_pins = 1;
+
+ for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
+ lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
+ "im-gpios", i);
+ if (lcd->im_pins[i] = -EPROBE_DEFER) {
+ dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
+ return -EPROBE_DEFER;
+ }
+ if (!gpio_is_valid(lcd->im_pins[i])) {
+ dev_err(&spi->dev, "Missing dt property: im-gpios\n");
+ return -EINVAL;
+ }
+
+ ret = devm_gpio_request_one(&spi->dev, lcd->im_pins[i],
+ GPIOF_OUT_INIT_LOW,
+ "im_pins");
+ if (ret) {
+ dev_err(&spi->dev, "failed to request gpio %d: %d\n",
+ lcd->im_pins[i], ret);
+ return -EINVAL;
+ }
}
-
- ret = devm_gpio_request_one(&spi->dev, lcd->im_pins[i],
- GPIOF_OUT_INIT_LOW, "im_pins");
- if (ret) {
- dev_err(&spi->dev, "failed to request gpio %d: %d\n",
- lcd->im_pins[i], ret);
- return -EINVAL;
- }
- }
+ } else
+ lcd->use_im_pins = 0;
lcdev = lcd_device_register("mxsfb", &spi->dev, lcd, &hx8357_ops);
if (IS_ERR(lcdev)) {
--
1.8.1.2
^ permalink raw reply related
* [PATCH 2/2] fb: backlight: HX8357: Add HX8369 support
From: Alexandre Belloni @ 2013-06-21 18:27 UTC (permalink / raw)
To: linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD
Cc: jimwall, brian, Maxime Ripard, linux-kernel, Richard Purdie,
Florian Tobias Schandinat, Alexandre Belloni
In-Reply-To: <1371839259-543-1-git-send-email-alexandre.belloni@free-electrons.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 183 +++++++++++++++++++++++++++++++++++++--
1 file changed, 175 insertions(+), 8 deletions(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 69f5672..b1c4042 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -71,6 +71,18 @@
#define HX8357_SET_POWER_NORMAL 0xd2
#define HX8357_SET_PANEL_RELATED 0xe9
+#define HX8369_SET_DISPLAY_BRIGHTNESS 0x51
+#define HX8369_WRITE_CABC_DISPLAY_VALUE 0x53
+#define HX8369_WRITE_CABC_BRIGHT_CTRL 0x55
+#define HX8369_WRITE_CABC_MIN_BRIGHTNESS 0x5e
+#define HX8369_SET_POWER 0xb1
+#define HX8369_SET_DISPLAY_MODE 0xb2
+#define HX8369_SET_DISPLAY_WAVEFORM_CYC 0xb4
+#define HX8369_SET_VCOM 0xb6
+#define HX8369_SET_EXTENSION_COMMAND 0xb9
+#define HX8369_SET_GIP 0xd5
+#define HX8369_SET_GAMMA_CURVE_RELATED 0xe0
+
struct hx8357_data {
unsigned im_pins[HX8357_NUM_IM_PINS];
unsigned reset;
@@ -144,6 +156,61 @@ static u8 hx8357_seq_display_mode[] = {
HX8357_SET_DISPLAY_MODE_RGB_INTERFACE,
};
+static u8 hx8369_seq_write_CABC_min_brightness[] = {
+ HX8369_WRITE_CABC_MIN_BRIGHTNESS, 0x00,
+};
+
+static u8 hx8369_seq_write_CABC_control[] = {
+ HX8369_WRITE_CABC_DISPLAY_VALUE, 0x24,
+};
+
+static u8 hx8369_seq_set_display_brightness[] = {
+ HX8369_SET_DISPLAY_BRIGHTNESS, 0xFF,
+};
+
+static u8 hx8369_seq_write_CABC_control_setting[] = {
+ HX8369_WRITE_CABC_BRIGHT_CTRL, 0x02,
+};
+
+static u8 hx8369_seq_extension_command[] = {
+ HX8369_SET_EXTENSION_COMMAND, 0xff, 0x83, 0x69,
+};
+
+static u8 hx8369_seq_display_related[] = {
+ HX8369_SET_DISPLAY_MODE, 0x00, 0x2b, 0x03, 0x03, 0x70, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x00, 0x03, 0x03, 0x00, 0x01,
+};
+
+static u8 hx8369_seq_panel_waveform_cycle[] = {
+ HX8369_SET_DISPLAY_WAVEFORM_CYC, 0x0a, 0x1d, 0x80, 0x06, 0x02,
+};
+
+static u8 hx8369_seq_set_address_mode[] = {
+ HX8357_SET_ADDRESS_MODE, 0x00,
+};
+
+static u8 hx8369_seq_vcom[] = {
+ HX8369_SET_VCOM, 0x3e, 0x3e,
+};
+
+static u8 hx8369_seq_gip[] = {
+ HX8369_SET_GIP, 0x00, 0x01, 0x03, 0x25, 0x01, 0x02, 0x28, 0x70,
+ 0x11, 0x13, 0x00, 0x00, 0x40, 0x26, 0x51, 0x37, 0x00, 0x00, 0x71,
+ 0x35, 0x60, 0x24, 0x07, 0x0f, 0x04, 0x04,
+};
+
+static u8 hx8369_seq_power[] = {
+ HX8369_SET_POWER, 0x01, 0x00, 0x34, 0x03, 0x00, 0x11, 0x11, 0x32,
+ 0x2f, 0x3f, 0x3f, 0x01, 0x3a, 0x01, 0xe6, 0xe6, 0xe6, 0xe6, 0xe6,
+};
+
+static u8 hx8369_seq_gamma_curve_related[] = {
+ HX8369_SET_GAMMA_CURVE_RELATED, 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d,
+ 0x2e, 0x4a, 0x08, 0x0e, 0x0f, 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
+ 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d, 0x2e, 0x4a, 0x08, 0x0e, 0x0f,
+ 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
+};
+
static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
u8 *txbuf, u16 txlen,
u8 *rxbuf, u16 rxlen)
@@ -242,6 +309,16 @@ static int hx8357_exit_standby(struct lcd_device *lcdev)
return 0;
}
+static void hx8357_lcd_reset(struct hx8357_data *lcd)
+{
+ gpio_set_value(lcd->reset, 1);
+ usleep_range(10000, 12000);
+ gpio_set_value(lcd->reset, 0);
+ usleep_range(10000, 12000);
+ gpio_set_value(lcd->reset, 1);
+ msleep(120);
+}
+
static int hx8357_lcd_init(struct lcd_device *lcdev)
{
struct hx8357_data *lcd = lcd_get_data(lcdev);
@@ -257,13 +334,7 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
gpio_set_value_cansleep(lcd->im_pins[2], 1);
}
- /* Reset the screen */
- gpio_set_value(lcd->reset, 1);
- usleep_range(10000, 12000);
- gpio_set_value(lcd->reset, 0);
- usleep_range(10000, 12000);
- gpio_set_value(lcd->reset, 1);
- msleep(120);
+ hx8357_lcd_reset(lcd);
ret = hx8357_spi_write_array(lcdev, hx8357_seq_power,
ARRAY_SIZE(hx8357_seq_power));
@@ -359,6 +430,97 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
return 0;
}
+static int hx8369_lcd_init(struct lcd_device *lcdev)
+{
+ struct hx8357_data *lcd = lcd_get_data(lcdev);
+ int ret;
+
+ hx8357_lcd_reset(lcd);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_extension_command,
+ ARRAY_SIZE(hx8369_seq_extension_command));
+ if (ret < 0)
+ return ret;
+ usleep_range(10000, 12000);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_display_related,
+ ARRAY_SIZE(hx8369_seq_display_related));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_panel_waveform_cycle,
+ ARRAY_SIZE(hx8369_seq_panel_waveform_cycle));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_address_mode,
+ ARRAY_SIZE(hx8369_seq_set_address_mode));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_vcom,
+ ARRAY_SIZE(hx8369_seq_vcom));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_gip,
+ ARRAY_SIZE(hx8369_seq_gip));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_power,
+ ARRAY_SIZE(hx8369_seq_power));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
+ if (ret < 0)
+ return ret;
+
+ msleep(120);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_gamma_curve_related,
+ ARRAY_SIZE(hx8369_seq_gamma_curve_related));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
+ if (ret < 0)
+ return ret;
+ usleep_range(1000, 1200);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_write_CABC_control,
+ ARRAY_SIZE(hx8369_seq_write_CABC_control));
+ if (ret < 0)
+ return ret;
+ usleep_range(10000, 12000);
+
+ ret = hx8357_spi_write_array(lcdev,
+ hx8369_seq_write_CABC_control_setting,
+ ARRAY_SIZE(hx8369_seq_write_CABC_control_setting));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev,
+ hx8369_seq_write_CABC_min_brightness,
+ ARRAY_SIZE(hx8369_seq_write_CABC_min_brightness));
+ if (ret < 0)
+ return ret;
+ usleep_range(10000, 12000);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_display_brightness,
+ ARRAY_SIZE(hx8369_seq_set_display_brightness));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_byte(lcdev, HX8357_SET_DISPLAY_ON);
+ if (ret < 0)
+ return ret;
+ msleep(100);
+
+ return 0;
+}
+
#define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
static int hx8357_set_power(struct lcd_device *lcdev, int power)
@@ -461,7 +623,11 @@ static int hx8357_probe(struct spi_device *spi)
}
spi_set_drvdata(spi, lcdev);
- ret = hx8357_lcd_init(lcdev);
+ ret = -ENODEV;
+ if (of_device_is_compatible(spi->dev.of_node, "himax,hx8357"))
+ ret = hx8357_lcd_init(lcdev);
+ else if (of_device_is_compatible(spi->dev.of_node, "himax,hx8369"))
+ ret = hx8369_lcd_init(lcdev);
if (ret) {
dev_err(&spi->dev, "Couldn't initialize panel\n");
goto init_error;
@@ -486,6 +652,7 @@ static int hx8357_remove(struct spi_device *spi)
static const struct of_device_id hx8357_dt_ids[] = {
{ .compatible = "himax,hx8357" },
+ { .compatible = "himax,hx8369" },
{},
};
MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 2/2] fb: backlight: HX8357: Add HX8369 support
From: Maxime Ripard @ 2013-06-21 18:49 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD, jimwall, brian,
linux-kernel, Richard Purdie, Florian Tobias Schandinat
In-Reply-To: <1371839259-543-2-git-send-email-alexandre.belloni@free-electrons.com>
[-- Attachment #1: Type: text/plain, Size: 8236 bytes --]
Hi Alex,
On Fri, Jun 21, 2013 at 08:27:39PM +0200, Alexandre Belloni wrote:
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/video/backlight/hx8357.c | 183 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 175 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
> index 69f5672..b1c4042 100644
> --- a/drivers/video/backlight/hx8357.c
> +++ b/drivers/video/backlight/hx8357.c
> @@ -71,6 +71,18 @@
> #define HX8357_SET_POWER_NORMAL 0xd2
> #define HX8357_SET_PANEL_RELATED 0xe9
>
> +#define HX8369_SET_DISPLAY_BRIGHTNESS 0x51
> +#define HX8369_WRITE_CABC_DISPLAY_VALUE 0x53
> +#define HX8369_WRITE_CABC_BRIGHT_CTRL 0x55
> +#define HX8369_WRITE_CABC_MIN_BRIGHTNESS 0x5e
> +#define HX8369_SET_POWER 0xb1
> +#define HX8369_SET_DISPLAY_MODE 0xb2
> +#define HX8369_SET_DISPLAY_WAVEFORM_CYC 0xb4
> +#define HX8369_SET_VCOM 0xb6
> +#define HX8369_SET_EXTENSION_COMMAND 0xb9
> +#define HX8369_SET_GIP 0xd5
> +#define HX8369_SET_GAMMA_CURVE_RELATED 0xe0
> +
> struct hx8357_data {
> unsigned im_pins[HX8357_NUM_IM_PINS];
> unsigned reset;
> @@ -144,6 +156,61 @@ static u8 hx8357_seq_display_mode[] = {
> HX8357_SET_DISPLAY_MODE_RGB_INTERFACE,
> };
>
> +static u8 hx8369_seq_write_CABC_min_brightness[] = {
> + HX8369_WRITE_CABC_MIN_BRIGHTNESS, 0x00,
> +};
> +
> +static u8 hx8369_seq_write_CABC_control[] = {
> + HX8369_WRITE_CABC_DISPLAY_VALUE, 0x24,
> +};
> +
> +static u8 hx8369_seq_set_display_brightness[] = {
> + HX8369_SET_DISPLAY_BRIGHTNESS, 0xFF,
> +};
> +
> +static u8 hx8369_seq_write_CABC_control_setting[] = {
> + HX8369_WRITE_CABC_BRIGHT_CTRL, 0x02,
> +};
> +
> +static u8 hx8369_seq_extension_command[] = {
> + HX8369_SET_EXTENSION_COMMAND, 0xff, 0x83, 0x69,
> +};
> +
> +static u8 hx8369_seq_display_related[] = {
> + HX8369_SET_DISPLAY_MODE, 0x00, 0x2b, 0x03, 0x03, 0x70, 0x00,
> + 0xff, 0x00, 0x00, 0x00, 0x00, 0x03, 0x03, 0x00, 0x01,
> +};
> +
> +static u8 hx8369_seq_panel_waveform_cycle[] = {
> + HX8369_SET_DISPLAY_WAVEFORM_CYC, 0x0a, 0x1d, 0x80, 0x06, 0x02,
> +};
> +
> +static u8 hx8369_seq_set_address_mode[] = {
> + HX8357_SET_ADDRESS_MODE, 0x00,
> +};
> +
> +static u8 hx8369_seq_vcom[] = {
> + HX8369_SET_VCOM, 0x3e, 0x3e,
> +};
> +
> +static u8 hx8369_seq_gip[] = {
> + HX8369_SET_GIP, 0x00, 0x01, 0x03, 0x25, 0x01, 0x02, 0x28, 0x70,
> + 0x11, 0x13, 0x00, 0x00, 0x40, 0x26, 0x51, 0x37, 0x00, 0x00, 0x71,
> + 0x35, 0x60, 0x24, 0x07, 0x0f, 0x04, 0x04,
> +};
> +
> +static u8 hx8369_seq_power[] = {
> + HX8369_SET_POWER, 0x01, 0x00, 0x34, 0x03, 0x00, 0x11, 0x11, 0x32,
> + 0x2f, 0x3f, 0x3f, 0x01, 0x3a, 0x01, 0xe6, 0xe6, 0xe6, 0xe6, 0xe6,
> +};
> +
> +static u8 hx8369_seq_gamma_curve_related[] = {
> + HX8369_SET_GAMMA_CURVE_RELATED, 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d,
> + 0x2e, 0x4a, 0x08, 0x0e, 0x0f, 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
> + 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d, 0x2e, 0x4a, 0x08, 0x0e, 0x0f,
> + 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
> +};
> +
> static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
> u8 *txbuf, u16 txlen,
> u8 *rxbuf, u16 rxlen)
> @@ -242,6 +309,16 @@ static int hx8357_exit_standby(struct lcd_device *lcdev)
> return 0;
> }
>
> +static void hx8357_lcd_reset(struct hx8357_data *lcd)
> +{
> + gpio_set_value(lcd->reset, 1);
> + usleep_range(10000, 12000);
> + gpio_set_value(lcd->reset, 0);
> + usleep_range(10000, 12000);
> + gpio_set_value(lcd->reset, 1);
> + msleep(120);
> +}
We should probably use the new reset framework here, especially
reset-gpio (I'm not sure what's the current status of the patch though).
> +
> static int hx8357_lcd_init(struct lcd_device *lcdev)
> {
> struct hx8357_data *lcd = lcd_get_data(lcdev);
> @@ -257,13 +334,7 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
> gpio_set_value_cansleep(lcd->im_pins[2], 1);
> }
>
> - /* Reset the screen */
> - gpio_set_value(lcd->reset, 1);
> - usleep_range(10000, 12000);
> - gpio_set_value(lcd->reset, 0);
> - usleep_range(10000, 12000);
> - gpio_set_value(lcd->reset, 1);
> - msleep(120);
> + hx8357_lcd_reset(lcd);
And that would become reset_device_reset
> ret = hx8357_spi_write_array(lcdev, hx8357_seq_power,
> ARRAY_SIZE(hx8357_seq_power));
> @@ -359,6 +430,97 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
> return 0;
> }
>
> +static int hx8369_lcd_init(struct lcd_device *lcdev)
> +{
> + struct hx8357_data *lcd = lcd_get_data(lcdev);
> + int ret;
> +
> + hx8357_lcd_reset(lcd);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_extension_command,
> + ARRAY_SIZE(hx8369_seq_extension_command));
> + if (ret < 0)
> + return ret;
> + usleep_range(10000, 12000);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_display_related,
> + ARRAY_SIZE(hx8369_seq_display_related));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_panel_waveform_cycle,
> + ARRAY_SIZE(hx8369_seq_panel_waveform_cycle));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_address_mode,
> + ARRAY_SIZE(hx8369_seq_set_address_mode));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_vcom,
> + ARRAY_SIZE(hx8369_seq_vcom));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_gip,
> + ARRAY_SIZE(hx8369_seq_gip));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_power,
> + ARRAY_SIZE(hx8369_seq_power));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
> + if (ret < 0)
> + return ret;
> +
> + msleep(120);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_gamma_curve_related,
> + ARRAY_SIZE(hx8369_seq_gamma_curve_related));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
> + if (ret < 0)
> + return ret;
> + usleep_range(1000, 1200);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_write_CABC_control,
> + ARRAY_SIZE(hx8369_seq_write_CABC_control));
> + if (ret < 0)
> + return ret;
> + usleep_range(10000, 12000);
> +
> + ret = hx8357_spi_write_array(lcdev,
> + hx8369_seq_write_CABC_control_setting,
> + ARRAY_SIZE(hx8369_seq_write_CABC_control_setting));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev,
> + hx8369_seq_write_CABC_min_brightness,
> + ARRAY_SIZE(hx8369_seq_write_CABC_min_brightness));
> + if (ret < 0)
> + return ret;
> + usleep_range(10000, 12000);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_display_brightness,
> + ARRAY_SIZE(hx8369_seq_set_display_brightness));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_byte(lcdev, HX8357_SET_DISPLAY_ON);
> + if (ret < 0)
> + return ret;
> + msleep(100);
> +
> + return 0;
> +}
> +
> #define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
>
> static int hx8357_set_power(struct lcd_device *lcdev, int power)
> @@ -461,7 +623,11 @@ static int hx8357_probe(struct spi_device *spi)
> }
> spi_set_drvdata(spi, lcdev);
>
> - ret = hx8357_lcd_init(lcdev);
> + ret = -ENODEV;
> + if (of_device_is_compatible(spi->dev.of_node, "himax,hx8357"))
> + ret = hx8357_lcd_init(lcdev);
> + else if (of_device_is_compatible(spi->dev.of_node, "himax,hx8369"))
> + ret = hx8369_lcd_init(lcdev);
> if (ret) {
> dev_err(&spi->dev, "Couldn't initialize panel\n");
> goto init_error;
> @@ -486,6 +652,7 @@ static int hx8357_remove(struct spi_device *spi)
>
> static const struct of_device_id hx8357_dt_ids[] = {
> { .compatible = "himax,hx8357" },
> + { .compatible = "himax,hx8369" },
Maybe you could use the .data field and of_match_device here.
> {},
> };
> MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
> --
> 1.8.1.2
>
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework
From: Jerome Glisse @ 2013-06-21 19:02 UTC (permalink / raw)
To: Inki Dae
Cc: Lucas Stach, linux-fbdev, Russell King - ARM Linux,
DRI mailing list, Kyungmin Park, myungjoo.ham, YoungJun Cho,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <CAAQKjZOeskLB7n6FM+bnB8n7ecuQM5k6uANXJXo=xk979f9s9Q@mail.gmail.com>
On Fri, Jun 21, 2013 at 12:55 PM, Inki Dae <daeinki@gmail.com> wrote:
> 2013/6/21 Lucas Stach <l.stach@pengutronix.de>:
>> Hi Inki,
>>
>> please refrain from sending HTML Mails, it makes proper quoting without
>> messing up the layout everywhere pretty hard.
>>
>
> Sorry about that. I should have used text mode.
>
>> Am Freitag, den 21.06.2013, 20:01 +0900 schrieb Inki Dae:
>> [...]
>>
>>> Yeah, you'll some knowledge and understanding about the API
>>> you are
>>> working with to get things right. But I think it's not an
>>> unreasonable
>>> thing to expect the programmer working directly with kernel
>>> interfaces
>>> to read up on how things work.
>>>
>>> Second thing: I'll rather have *one* consistent API for every
>>> subsystem,
>>> even if they differ from each other than having to implement
>>> this
>>> syncpoint thing in every subsystem. Remember: a single execbuf
>>> in DRM
>>> might reference both GEM objects backed by dma-buf as well
>>> native SHM or
>>> CMA backed objects. The dma-buf-mgr proposal already allows
>>> you to
>>> handle dma-bufs much the same way during validation than
>>> native GEM
>>> objects.
>>>
>>> Actually, at first I had implemented a fence helper framework based on
>>> reservation and dma fence to provide easy-use-interface for device
>>> drivers. However, that was wrong implemention: I had not only
>>> customized the dma fence but also not considered dead lock issue.
>>> After that, I have reimplemented it as dmabuf sync to solve dead
>>> issue, and at that time, I realized that we first need to concentrate
>>> on the most basic thing: the fact CPU and CPU, CPU and DMA, or DMA and
>>> DMA can access a same buffer, And the fact simple is the best, and the
>>> fact we need not only kernel side but also user side interfaces. After
>>> that, I collected what is the common part for all subsystems, and I
>>> have devised this dmabuf sync framework for it. I'm not really
>>> specialist in Desktop world. So question. isn't the execbuf used only
>>> for the GPU? the gpu has dedicated video memory(VRAM) so it needs
>>> migration mechanism between system memory and the dedicated video
>>> memory, and also to consider ordering issue while be migrated.
>>>
>>
>> Yeah, execbuf is pretty GPU specific, but I don't see how this matters
>> for this discussion. Also I don't see a big difference between embedded
>> and desktop GPUs. Buffer migration is more of a detail here. Both take
>> command stream that potentially reference other buffers, which might be
>> native GEM or dma-buf backed objects. Both have to make sure the buffers
>> are in the right domain (caches cleaned and address mappings set up) and
>> are available for the desired operation, meaning you have to sync with
>> other DMA engines and maybe also with CPU.
>
> Yeah, right. Then, in case of desktop gpu, does't it need additional
> something to do when a buffer/s is/are migrated from system memory to
> video memory domain, or from video memory to system memory domain? I
> guess the below members does similar thing, and all other DMA devices
> would not need them:
> struct fence {
> ...
> unsigned int context, seqno;
> ...
> };
>
> And,
> struct seqno_fence {
> ...
> uint32_t seqno_ofs;
> ...
> };
>
>>
>> The only case where sync isn't clearly defined right now by the current
>> API entrypoints is when you access memory through the dma-buf fallback
>> mmap support, which might happen with some software processing element
>> in a video pipeline or something. I agree that we will need a userspace
>> interface here, but I think this shouldn't be yet another sync object,
>> but rather more a prepare/fini_cpu_access ioctl on the dma-buf which
>> hooks into the existing dma-fence and reservation stuff.
>
> I think we don't need addition ioctl commands for that. I am thinking
> of using existing resources as possible. My idea also is similar in
> using the reservation stuff to your idea because my approach also
> should use the dma-buf resource. However, My idea is that a user
> process, that wants buffer synchronization with the other, sees a sync
> object as a file descriptor like dma-buf does. The below shows simple
> my idea about it:
>
> ioctl(dmabuf_fd, DMA_BUF_IOC_OPEN_SYNC, &sync);
>
> flock(sync->fd, LOCK_SH); <- LOCK_SH means a shared lock.
> CPU access for read
> flock(sync->fd, LOCK_UN);
>
> Or
>
> flock(sync->fd, LOCK_EX); <- LOCK_EX means an exclusive lock
> CPU access for write
> flock(sync->fd, LOCK_UN);
>
> close(sync->fd);
>
> As you know, that's similar to dmabuf export feature.
>
> In addition, a more simple idea,
> flock(dmabuf_fd, LOCK_SH/EX);
> CPU access for read/write
> flock(dmabuf_fd, LOCK_UN);
>
> However, I'm not sure that the above examples could be worked well,
> and there are no problems yet: actually, I don't fully understand
> flock mechanism, so looking into it.
>
>>
>>>
>>> And to get back to my original point: if you have more than
>>> one task
>>> operating together on a buffer you absolutely need some kind
>>> of real IPC
>>> to sync them up and do something useful. Both you syncpoints
>>> and the
>>> proposed dma-fences only protect the buffer accesses to make
>>> sure
>>> different task don't stomp on each other. There is nothing in
>>> there to
>>> make sure that the output of your pipeline is valid. You have
>>> to take
>>> care of that yourself in userspace. I'll reuse your example to
>>> make it
>>> clear what I mean:
>>>
>>> Task A Task B
>>> ------ -------
>>> dma_buf_sync_lock(buf1)
>>> CPU write buf1
>>> dma_buf_sync_unlock(buf1)
>>> ---------schedule Task A again-------
>>> dma_buf_sync_lock(buf1)
>>> CPU write buf1
>>> dma_buf_sync_unlock(buf1)
>>> ---------schedule Task B---------
>>> qbuf(buf1)
>>>
>>> dma_buf_sync_lock(buf1)
>>> ....
>>>
>>> This is what can happen if you don't take care of proper
>>> syncing. Task A
>>> writes something to the buffer in expectation that Task B will
>>> take care
>>> of it, but before Task B even gets scheduled Task A overwrites
>>> the
>>> buffer again. Not what you wanted, isn't it?
>>>
>>> Exactly wrong example. I had already mentioned about that. "In case
>>> that data flow goes from A to B, it needs some kind of IPC between the
>>> two tasks every time" So again, your example would have no any
>>> problem in case that *two tasks share the same buffer but these tasks
>>> access the buffer(buf1) as write, and data of the buffer(buf1) isn't
>>> needed to be shared*. They just need to use the buffer as *storage*.
>>> So All they want is to avoid stomping on the buffer in this case.
>>>
>> Sorry, but I don't see the point. If no one is interested in the data of
>> the buffer, why are you sharing it in the first place?
>>
>
> Just used as a storage. i.e., Task A fills the buffer with "AAAAAA"
> using CPU, And Task B fills the buffer with "BBBBBB" using DMA. They
> don't share data of the buffer, but they share *memory region* of the
> buffer. That would be very useful for the embedded systems with very
> small size system memory.
Just so i understand. You want to share backing memory, you don't want
to share content ie you want to do memory management in userspace.
This sounds wrong on so many level (not even considering the security
implication).
If Task A need memory and then can release it for Task B usage that
should be the role of kernel memory management which of course needs
synchronization btw A and B. But in no case this should be done using
dma-buf. dma-buf is for sharing content btw different devices not
sharing resources.
Also don't over complicate the vram case, just consider desktop gpu as
using system memory directly. They can do it and they do it. Migration
to vram is orthogonal to all this, it's an optimization so to speak.
Cheers,
Jerome
^ permalink raw reply
* Change the framebuffer background color
From: Дмитрий Головин @ 2013-06-23 11:37 UTC (permalink / raw)
To: linux-fbdev
Dear All!
I want to change the color with which the screen is filled when framebuffer device initializes.
I posted a question on StackOverflow (http://stackoverflow.com/questions/17252906), but nobody answered so far.
I'll just quote it:
> When framebuffer device initializes (I guess it is vesafb), the screen is filled with black color. How can I change that color?
Thank you in advance for your answers.
Regards,
Dmitry
^ permalink raw reply
* Re: MMP display subsystem questions
From: Zhou Zhu @ 2013-06-24 7:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMLZHHQFtXWx0weG3uh14Nm3euqbSqyztBEQHEQFBzjFxWGTZA@mail.gmail.com>
Daniel,
Thank you for reviewing our patches.
Please see my comments.
On 06/22/2013 12:14 AM, Daniel Drake wrote:
>>> 4. Which overlay to use - according to my investigation above, this
>>> doesn't actually have any meaningful effect on the driver.
>>
>> As overlay enabled in patch above now, this config is required to make fb
>> show on some overlay and other interface(v4l2, private interface) to use
>> others.
>
> I don't understand this, but it might be easier to understand once you
> have documented what an overlay is and how it relates to a path. How
> is v4l2 going to interact with this driver? What private interfaces
> are you referring to?
>
> Or can we just auto-instantiate one framebuffer per path with good defaults?
Actually besides fb, v4l2 and an IOCTL based interface to support what
fb/v4l2 not provided are also added. The v4l2 or private interface could
also directly use same interface defined in include/video/mmp_disp.h to
get path/overlay and manipulate the hardware.
Patches for these interfaces would be submitted later and documentation
would also be added.
>
>> Also there still might be some settings in fb for some further settings like
>> vsync usage or yres_virtual size in coming patches.
>
> Wouldn't such information be represented in the modes supported by the panel?
These settings are software related rather than what hw supports.
Vsync settings include whether fb wait vsync interrupt to sync buffers
when pan display or whether send uevent in vsync (which is a mechanism
for Android).
For yres_virtual size which is related with pre-reserved fb buffer size
and some buffer sync mechanism.
As these settings are pure fb related so we just not place it in panel.
>
> A couple more questions:
>
> 1. What is the invert_pixclock setting in the mmp_mode structure? This
> causes a currently unused bit to be set in fb_videomode->vmode to be
> set (seems dangerous). And then nothing acts upon this bit, as far as
> I can see.
This bit is a legacy settings that for some panels that require inverted
pixel clock. However, as panels we are using now don't require such
feature and even we forget it by mistake, we could make a patch to
remove it and related code soon later.
Thank you for pointing out this:)
>
> 2. What about pix_fmt_out? Why is pixel format dumped into the mode
> specifier? If there is a supported panel that really can support
> different pixel formats, I would expect it to support all the pixel
> formats for all the resolution. This also seems unused within the
> driver.
We added it due to 2 reasons:
1. It impacts output timing settings inside controller for dsi case (we
have already in-home patches and would be submitted later). So it's
required to indicate that timing need to be changed.
2. It's still possible that for some panel that could support different
resolution and pixel formats, but it might not support all pixel formats
for all resolutions.
Also this variable is not used for dumb panel as it's already covered in
dumb mode in link config - dumb mode includes not only 16bpp or 24bpp
but also whether low 16 bits or high 16 bits used in 24 bits' lines. For
coming DSI panel, it's required and would be used.
>
> Thanks
> Daniel
>
--
Thanks, -Zhou
^ permalink raw reply
* Re: [V2 1/7] video: mmp: rb swap setting update for new LCD driver
From: jett zhou @ 2013-06-24 10:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMLZHHQ+97f=i3axaFBn9JoOu7Cqo73neqxKYNCfWqwjZ4DfnQ@mail.gmail.com>
2013/6/22 Daniel Drake <dsd@laptop.org>:
> On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
>> From: Guoqing Li <ligq@marvell.com>
>>
>> We could set rb swap in two modules: DMA controler and interface
>> output after blending.
>> This patch move the panel rbswap requirement setting in later module.
>
> link_config originates from the platform's path config. link_config is
> undocumented and this patch also changes its meaning.
>
> Previously, bit 0 triggered rbswap, and this behaviour is relied upon
> by arch/arm/mach-mmp/ttc_dkb.c
>
> Now bits 27:24 of link_config are used to enable or disable rbswap,
> and link_config bit 0 is ignored. According to the specs for the panel
> path register, valid values for these new bits are 0 (no swap) and 1
> (swap). ttc_dkb has not been updated in this patch series for this new
> behaviour.
>
> I don't understand why rbswap is set to 1 in fmt_to_reg for certain
> RGB formats. The patch description suggests that the rb swapping can
> either be set in fmt_to_reg context (to be written into DMA
> controller) *or* in the interface output. If we are now relying on the
> interface output control to do RB swapping when appropriate, why are
> there still cases when we ask the DMA controller to do the same thing?
>
> I also do not fully understand the requirement for RB swapping. I do
> understand that it changes pixel format from RGB to BGR. What is the
> point? I can imagine that some panels may require such a pixel format,
> however in that case, this configuration should be part of the panel
> configuration, not part of the path configuration as it currently is.
>
> Daniel
Hi Daniel
Sorry that the comments of the patch is not so clear.
As you might know,We can set two rbswap setting, one is based
on pix_fmt to set into DMA input part, the other is to configured in
the output interface part.
This patch include below change and enhancement:
1) The input format which support rbswap is based RGB format,
YUV series did not support now. So you can see the patch will change
the rbswap value based on specific RGB format,eg. RGB will set rbswap
= 1, BGR will not set it.
This part setting is controlled in driver internally, will
not depend on platform configure(link_config) any more.
2) The output part of rbswap is depend on link_config which is
defined in specific platfrom.
It will setting bit27:24 of 0x01DC register, eg. DSI output
interface can be controlled by this.
TTC_dkb does not support dsi, the link_config is no used anymore.
I will add more this comments in the patch of V3.
Thanks
--
----------------------------------
Best Regards
Jett Zhou
^ permalink raw reply
* Re: [V2 3/7] video: mmp: fix graphics/video layer enable/mask swap issue
From: jett zhou @ 2013-06-24 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMLZHHSDUYZ8fpjjSKzDWKL3=RhOaxO8pq-ty5zM+KHzEu30QA@mail.gmail.com>
2013/6/22 Daniel Drake <dsd@laptop.org>:
> On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
>> From: Jing Xiang <jxiang@marvell.com>
>>
>> There is bug when switch dma of graphic layer and video layer, it
>> configured opposite bit, fix it.
>
> So, overlays can be either graphics or video. overlay_is_vid() tells
> you which type.
> overlay_is_vid() makes its decision based on dmafetch_id which is
> undocumented and I don't understand it (and in another mail you said
> this is legacy configuration that is going to be removed).
>
> Ignoring my lack of understanding of the background: the patch here
> seems to make the code more correct. It will modify the path's
> underlying control register to enable graphics or video transfer
> depending on the overlay type. Turning a graphics overlay dmafetch ON
> will not stomp on the bits set by any video overlay dmafetch on the
> same path. Previously the logic was reversed.
>
> This codepath will not work correctly if there will ever be more than
> one overlay of the same type on the same path. For example, if we have
> two graphics overlays;
> 1. dmafetch_onoff(overlay1, on) - graphics transfer is now enabled in
> the hardware
> 2. dmafetch_onoff(overlay2, on) - graphics transfer was already
> enabled, no change
> 3. dmafetch_onoff(overlay2, off) - graphics transfer is now disabled
> in the hardware
>
> At this point overlay1 is still active in theory, but the driver has
> disabled graphics transfer at the hardware level.
>
> Daniel
Hi Daniel
Now for our controller and usage, we have 2 over-lay of the path,
they are graphic layer and video layer.
dmafetch_id =0 if it is graphic layer, dmafetch_id =1 if it is video layer.
The another mail mentioned is right, we plan to remove the
dmafetch_id and detect overlay_is_vid by other method.
But this patch did not include it yet, it will be included by
another patch later on.
Thanks
--
----------------------------------
Best Regards
Jett Zhou
^ permalink raw reply
* Re: [V2 5/7] video: mmp: add pitch info in mmp_win structure
From: jett zhou @ 2013-06-24 10:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMLZHHRxS-wqd4Tb=Af6VEnfRb9MJeNo5RC7qeTLBZ0rgU69pw@mail.gmail.com>
2013/6/22 Daniel Drake <dsd@laptop.org>:
> On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou <jtzhou@marvell.com> wrote:
>> From: Jing Xiang <jxiang@marvell.com>
>>
>> Add pitch length info of graphics/video layer for mmp_win, if it is
>> YUV format of video layer, u/v pitch will non-zero.
>>
>> Signed-off-by: Jing Xiang <jxiang@marvell.com>
>> Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
>> ---
>> include/video/mmp_disp.h | 5 +++++
>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
>> index b9dd1fb..462e3bd 100644
>> --- a/include/video/mmp_disp.h
>> +++ b/include/video/mmp_disp.h
>> @@ -91,6 +91,11 @@ struct mmp_win {
>> u16 up_crop;
>> u16 bottom_crop;
>> int pix_fmt;
>> + /*
>> + * pitch[0]: graphics/video layer line length or y pitch
>> + * pitch[1]/pitch[2]: video u/v pitch if non-zero
>> + */
>> + u32 pitch[3];
>> };
>
> Thanks for adding a comment here, but the meaning of this field is
> still not clear to me.
> In what case is pitch[0] line length, and in which case does it refer
> to y pitch?
>
> pitch[1] and pitch[2] refer to u/v pitch respectively, if their own
> values are non-zero? (or if not, what value does the "if non-zero"
> comment refer to?)
>
> I would recommend rolling this patch into the patch that actually
> makes use of this new field.
>
> Daniel
Hi Daniel
pitch is used to represent line length in byte, the usage depends
on pix_fmt.
If the fmt is YUV , the pitch[0] will be Y length, pitch[1] will
be U length, pitch[2] will be V lenth.
If the fmt is RGB, the picth[0] will be line lenth, and
pitch[1]/pitch[2] will be 0 and not be used.
You can refer to pixfmt_to_stride func implementation.
For the patch rolling, do you mean combine the patch5 and patch6
by one patch?
Thanks
--
----------------------------------
Best Regards
Jett Zhou
^ permalink raw reply
* [PATCHv2 0/2] HX8357 driver improvement
From: Alexandre Belloni @ 2013-06-24 14:16 UTC (permalink / raw)
To: linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD
Cc: jimwall, brian, Maxime Ripard, linux-kernel, Richard Purdie,
Florian Tobias Schandinat, Alexandre Belloni
Hi,
This is a set of improvements for the hx8357 driver. The first patch removes the
need to defines IM pins in the device tree. So that it is possible to use the
controller with fixed IM pins.
The second patch adds support for the Himax HX8369 controller which is quite
similar.
Tested on CFA-10055 (hx8357) and CFA-10056 (hx8369).
Alexandre Belloni (1):
fb: backlight: HX8357: Add HX8369 support
Maxime Ripard (1):
video: hx8357: Make IM pins optional
drivers/video/backlight/hx8357.c | 256 +++++++++++++++++++++++++++++++++------
1 file changed, 219 insertions(+), 37 deletions(-)
--
1.8.1.2
^ permalink raw reply
* [PATCHv2 1/2] video: hx8357: Make IM pins optional
From: Alexandre Belloni @ 2013-06-24 14:16 UTC (permalink / raw)
To: linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD
Cc: jimwall, brian, Maxime Ripard, linux-kernel, Richard Purdie,
Florian Tobias Schandinat
In-Reply-To: <1372083415-9734-1-git-send-email-alexandre.belloni@free-electrons.com>
From: Maxime Ripard <maxime.ripard@free-electrons.com>
The IM pins of the HX8357 controller are used to define the interface used to
feed pixel stream to the LCD panel.
Most of the time, these pins are directly routed to either the ground or the VCC
to set their values.
Remove the need to assign GPIOs to these pins when we are in such a case.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 53 +++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index a0482b5..69f5672 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -76,6 +76,7 @@ struct hx8357_data {
unsigned reset;
struct spi_device *spi;
int state;
+ u8 use_im_pins;
};
static u8 hx8357_seq_power[] = {
@@ -250,9 +251,11 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
* Set the interface selection pins to SPI mode, with three
* wires
*/
- gpio_set_value_cansleep(lcd->im_pins[0], 1);
- gpio_set_value_cansleep(lcd->im_pins[1], 0);
- gpio_set_value_cansleep(lcd->im_pins[2], 1);
+ if (lcd->use_im_pins) {
+ gpio_set_value_cansleep(lcd->im_pins[0], 1);
+ gpio_set_value_cansleep(lcd->im_pins[1], 0);
+ gpio_set_value_cansleep(lcd->im_pins[2], 1);
+ }
/* Reset the screen */
gpio_set_value(lcd->reset, 1);
@@ -424,26 +427,32 @@ static int hx8357_probe(struct spi_device *spi)
return -EINVAL;
}
- for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
- lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
- "im-gpios", i);
- if (lcd->im_pins[i] = -EPROBE_DEFER) {
- dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
- return -EPROBE_DEFER;
- }
- if (!gpio_is_valid(lcd->im_pins[i])) {
- dev_err(&spi->dev, "Missing dt property: im-gpios\n");
- return -EINVAL;
+ if (of_find_property(spi->dev.of_node, "im-gpios", NULL)) {
+ lcd->use_im_pins = 1;
+
+ for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
+ lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
+ "im-gpios", i);
+ if (lcd->im_pins[i] = -EPROBE_DEFER) {
+ dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
+ return -EPROBE_DEFER;
+ }
+ if (!gpio_is_valid(lcd->im_pins[i])) {
+ dev_err(&spi->dev, "Missing dt property: im-gpios\n");
+ return -EINVAL;
+ }
+
+ ret = devm_gpio_request_one(&spi->dev, lcd->im_pins[i],
+ GPIOF_OUT_INIT_LOW,
+ "im_pins");
+ if (ret) {
+ dev_err(&spi->dev, "failed to request gpio %d: %d\n",
+ lcd->im_pins[i], ret);
+ return -EINVAL;
+ }
}
-
- ret = devm_gpio_request_one(&spi->dev, lcd->im_pins[i],
- GPIOF_OUT_INIT_LOW, "im_pins");
- if (ret) {
- dev_err(&spi->dev, "failed to request gpio %d: %d\n",
- lcd->im_pins[i], ret);
- return -EINVAL;
- }
- }
+ } else
+ lcd->use_im_pins = 0;
lcdev = lcd_device_register("mxsfb", &spi->dev, lcd, &hx8357_ops);
if (IS_ERR(lcdev)) {
--
1.8.1.2
^ permalink raw reply related
* [PATCHv2 2/2] fb: backlight: HX8357: Add HX8369 support
From: Alexandre Belloni @ 2013-06-24 14:16 UTC (permalink / raw)
To: linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD
Cc: jimwall, brian, Maxime Ripard, linux-kernel, Richard Purdie,
Florian Tobias Schandinat, Alexandre Belloni
In-Reply-To: <1372083415-9734-1-git-send-email-alexandre.belloni@free-electrons.com>
Add support for the Himax HX8369 controller as it is quite similar to the
hx8357.
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
changes since v1:
- use the data element of of_device_id to select the correct initialization
function
drivers/video/backlight/hx8357.c | 203 ++++++++++++++++++++++++++++++++++++---
1 file changed, 188 insertions(+), 15 deletions(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 69f5672..496cb58 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -71,6 +71,18 @@
#define HX8357_SET_POWER_NORMAL 0xd2
#define HX8357_SET_PANEL_RELATED 0xe9
+#define HX8369_SET_DISPLAY_BRIGHTNESS 0x51
+#define HX8369_WRITE_CABC_DISPLAY_VALUE 0x53
+#define HX8369_WRITE_CABC_BRIGHT_CTRL 0x55
+#define HX8369_WRITE_CABC_MIN_BRIGHTNESS 0x5e
+#define HX8369_SET_POWER 0xb1
+#define HX8369_SET_DISPLAY_MODE 0xb2
+#define HX8369_SET_DISPLAY_WAVEFORM_CYC 0xb4
+#define HX8369_SET_VCOM 0xb6
+#define HX8369_SET_EXTENSION_COMMAND 0xb9
+#define HX8369_SET_GIP 0xd5
+#define HX8369_SET_GAMMA_CURVE_RELATED 0xe0
+
struct hx8357_data {
unsigned im_pins[HX8357_NUM_IM_PINS];
unsigned reset;
@@ -144,6 +156,61 @@ static u8 hx8357_seq_display_mode[] = {
HX8357_SET_DISPLAY_MODE_RGB_INTERFACE,
};
+static u8 hx8369_seq_write_CABC_min_brightness[] = {
+ HX8369_WRITE_CABC_MIN_BRIGHTNESS, 0x00,
+};
+
+static u8 hx8369_seq_write_CABC_control[] = {
+ HX8369_WRITE_CABC_DISPLAY_VALUE, 0x24,
+};
+
+static u8 hx8369_seq_set_display_brightness[] = {
+ HX8369_SET_DISPLAY_BRIGHTNESS, 0xFF,
+};
+
+static u8 hx8369_seq_write_CABC_control_setting[] = {
+ HX8369_WRITE_CABC_BRIGHT_CTRL, 0x02,
+};
+
+static u8 hx8369_seq_extension_command[] = {
+ HX8369_SET_EXTENSION_COMMAND, 0xff, 0x83, 0x69,
+};
+
+static u8 hx8369_seq_display_related[] = {
+ HX8369_SET_DISPLAY_MODE, 0x00, 0x2b, 0x03, 0x03, 0x70, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x00, 0x03, 0x03, 0x00, 0x01,
+};
+
+static u8 hx8369_seq_panel_waveform_cycle[] = {
+ HX8369_SET_DISPLAY_WAVEFORM_CYC, 0x0a, 0x1d, 0x80, 0x06, 0x02,
+};
+
+static u8 hx8369_seq_set_address_mode[] = {
+ HX8357_SET_ADDRESS_MODE, 0x00,
+};
+
+static u8 hx8369_seq_vcom[] = {
+ HX8369_SET_VCOM, 0x3e, 0x3e,
+};
+
+static u8 hx8369_seq_gip[] = {
+ HX8369_SET_GIP, 0x00, 0x01, 0x03, 0x25, 0x01, 0x02, 0x28, 0x70,
+ 0x11, 0x13, 0x00, 0x00, 0x40, 0x26, 0x51, 0x37, 0x00, 0x00, 0x71,
+ 0x35, 0x60, 0x24, 0x07, 0x0f, 0x04, 0x04,
+};
+
+static u8 hx8369_seq_power[] = {
+ HX8369_SET_POWER, 0x01, 0x00, 0x34, 0x03, 0x00, 0x11, 0x11, 0x32,
+ 0x2f, 0x3f, 0x3f, 0x01, 0x3a, 0x01, 0xe6, 0xe6, 0xe6, 0xe6, 0xe6,
+};
+
+static u8 hx8369_seq_gamma_curve_related[] = {
+ HX8369_SET_GAMMA_CURVE_RELATED, 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d,
+ 0x2e, 0x4a, 0x08, 0x0e, 0x0f, 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
+ 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d, 0x2e, 0x4a, 0x08, 0x0e, 0x0f,
+ 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
+};
+
static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
u8 *txbuf, u16 txlen,
u8 *rxbuf, u16 rxlen)
@@ -242,6 +309,18 @@ static int hx8357_exit_standby(struct lcd_device *lcdev)
return 0;
}
+static void hx8357_lcd_reset(struct lcd_device *lcdev)
+{
+ struct hx8357_data *lcd = lcd_get_data(lcdev);
+
+ gpio_set_value(lcd->reset, 1);
+ usleep_range(10000, 12000);
+ gpio_set_value(lcd->reset, 0);
+ usleep_range(10000, 12000);
+ gpio_set_value(lcd->reset, 1);
+ msleep(120);
+}
+
static int hx8357_lcd_init(struct lcd_device *lcdev)
{
struct hx8357_data *lcd = lcd_get_data(lcdev);
@@ -257,14 +336,6 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
gpio_set_value_cansleep(lcd->im_pins[2], 1);
}
- /* Reset the screen */
- gpio_set_value(lcd->reset, 1);
- usleep_range(10000, 12000);
- gpio_set_value(lcd->reset, 0);
- usleep_range(10000, 12000);
- gpio_set_value(lcd->reset, 1);
- msleep(120);
-
ret = hx8357_spi_write_array(lcdev, hx8357_seq_power,
ARRAY_SIZE(hx8357_seq_power));
if (ret < 0)
@@ -359,6 +430,94 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
return 0;
}
+static int hx8369_lcd_init(struct lcd_device *lcdev)
+{
+ int ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_extension_command,
+ ARRAY_SIZE(hx8369_seq_extension_command));
+ if (ret < 0)
+ return ret;
+ usleep_range(10000, 12000);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_display_related,
+ ARRAY_SIZE(hx8369_seq_display_related));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_panel_waveform_cycle,
+ ARRAY_SIZE(hx8369_seq_panel_waveform_cycle));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_address_mode,
+ ARRAY_SIZE(hx8369_seq_set_address_mode));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_vcom,
+ ARRAY_SIZE(hx8369_seq_vcom));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_gip,
+ ARRAY_SIZE(hx8369_seq_gip));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_power,
+ ARRAY_SIZE(hx8369_seq_power));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
+ if (ret < 0)
+ return ret;
+
+ msleep(120);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_gamma_curve_related,
+ ARRAY_SIZE(hx8369_seq_gamma_curve_related));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
+ if (ret < 0)
+ return ret;
+ usleep_range(1000, 1200);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_write_CABC_control,
+ ARRAY_SIZE(hx8369_seq_write_CABC_control));
+ if (ret < 0)
+ return ret;
+ usleep_range(10000, 12000);
+
+ ret = hx8357_spi_write_array(lcdev,
+ hx8369_seq_write_CABC_control_setting,
+ ARRAY_SIZE(hx8369_seq_write_CABC_control_setting));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_array(lcdev,
+ hx8369_seq_write_CABC_min_brightness,
+ ARRAY_SIZE(hx8369_seq_write_CABC_min_brightness));
+ if (ret < 0)
+ return ret;
+ usleep_range(10000, 12000);
+
+ ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_display_brightness,
+ ARRAY_SIZE(hx8369_seq_set_display_brightness));
+ if (ret < 0)
+ return ret;
+
+ ret = hx8357_spi_write_byte(lcdev, HX8357_SET_DISPLAY_ON);
+ if (ret < 0)
+ return ret;
+ msleep(100);
+
+ return 0;
+}
+
#define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
static int hx8357_set_power(struct lcd_device *lcdev, int power)
@@ -391,10 +550,24 @@ static struct lcd_ops hx8357_ops = {
.get_power = hx8357_get_power,
};
+static const struct of_device_id hx8357_dt_ids[] = {
+ {
+ .compatible = "himax,hx8357",
+ .data = hx8357_lcd_init,
+ },
+ {
+ .compatible = "himax,hx8369",
+ .data = hx8369_lcd_init,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
+
static int hx8357_probe(struct spi_device *spi)
{
struct lcd_device *lcdev;
struct hx8357_data *lcd;
+ const struct of_device_id *match;
int i, ret;
lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL);
@@ -411,6 +584,10 @@ static int hx8357_probe(struct spi_device *spi)
lcd->spi = spi;
+ match = of_match_device(hx8357_dt_ids, &spi->dev);
+ if (!match || !match->data)
+ return -EINVAL;
+
lcd->reset = of_get_named_gpio(spi->dev.of_node, "gpios-reset", 0);
if (!gpio_is_valid(lcd->reset)) {
dev_err(&spi->dev, "Missing dt property: gpios-reset\n");
@@ -461,7 +638,9 @@ static int hx8357_probe(struct spi_device *spi)
}
spi_set_drvdata(spi, lcdev);
- ret = hx8357_lcd_init(lcdev);
+ hx8357_lcd_reset(lcdev);
+
+ ret = ((int (*)(struct lcd_device *))match->data)(lcdev);
if (ret) {
dev_err(&spi->dev, "Couldn't initialize panel\n");
goto init_error;
@@ -484,12 +663,6 @@ static int hx8357_remove(struct spi_device *spi)
return 0;
}
-static const struct of_device_id hx8357_dt_ids[] = {
- { .compatible = "himax,hx8357" },
- {},
-};
-MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
-
static struct spi_driver hx8357_driver = {
.probe = hx8357_probe,
.remove = hx8357_remove,
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH 1/2] fb: backlight: HX8357: Make IM pins optionnal
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-24 14:26 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-fbdev, jimwall, brian, Maxime Ripard, linux-kernel,
Richard Purdie, Florian Tobias Schandinat
In-Reply-To: <1371839259-543-1-git-send-email-alexandre.belloni@free-electrons.com>
On 20:27 Fri 21 Jun , Alexandre Belloni wrote:
> From: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> drivers/video/backlight/hx8357.c | 53 +++++++++++++++++++++++-----------------
> 1 file changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
> index a0482b5..69f5672 100644
> --- a/drivers/video/backlight/hx8357.c
> +++ b/drivers/video/backlight/hx8357.c
> @@ -76,6 +76,7 @@ struct hx8357_data {
> unsigned reset;
> struct spi_device *spi;
> int state;
> + u8 use_im_pins;
boolean please
> };
>
> static u8 hx8357_seq_power[] = {
> @@ -250,9 +251,11 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
> * Set the interface selection pins to SPI mode, with three
> * wires
> */
> - gpio_set_value_cansleep(lcd->im_pins[0], 1);
> - gpio_set_value_cansleep(lcd->im_pins[1], 0);
> - gpio_set_value_cansleep(lcd->im_pins[2], 1);
> + if (lcd->use_im_pins) {
> + gpio_set_value_cansleep(lcd->im_pins[0], 1);
> + gpio_set_value_cansleep(lcd->im_pins[1], 0);
> + gpio_set_value_cansleep(lcd->im_pins[2], 1);
> + }
base on the dt probe you may have gpios betwee 0 to HX8357_NUM_IM_PINS
so this look wrong
>
> /* Reset the screen */
> gpio_set_value(lcd->reset, 1);
> @@ -424,26 +427,32 @@ static int hx8357_probe(struct spi_device *spi)
> return -EINVAL;
> }
>
> - for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
> - lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
> - "im-gpios", i);
> - if (lcd->im_pins[i] = -EPROBE_DEFER) {
> - dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
> - return -EPROBE_DEFER;
> - }
> - if (!gpio_is_valid(lcd->im_pins[i])) {
> - dev_err(&spi->dev, "Missing dt property: im-gpios\n");
> - return -EINVAL;
> + if (of_find_property(spi->dev.of_node, "im-gpios", NULL)) {
> + lcd->use_im_pins = 1;
> +
> + for (i = 0; i < HX8357_NUM_IM_PINS; i++) {
> + lcd->im_pins[i] = of_get_named_gpio(spi->dev.of_node,
> + "im-gpios", i);
> + if (lcd->im_pins[i] = -EPROBE_DEFER) {
> + dev_info(&spi->dev, "GPIO requested is not here yet, deferring the probe\n");
> + return -EPROBE_DEFER;
> + }
> + if (!gpio_is_valid(lcd->im_pins[i])) {
> + dev_err(&spi->dev, "Missing dt property: im-gpios\n");
> + return -EINVAL;
> + }
> +
> + ret = devm_gpio_request_one(&spi->dev, lcd->im_pins[i],
> + GPIOF_OUT_INIT_LOW,
> + "im_pins");
> + if (ret) {
> + dev_err(&spi->dev, "failed to request gpio %d: %d\n",
> + lcd->im_pins[i], ret);
> + return -EINVAL;
> + }
> }
> -
> - ret = devm_gpio_request_one(&spi->dev, lcd->im_pins[i],
> - GPIOF_OUT_INIT_LOW, "im_pins");
> - if (ret) {
> - dev_err(&spi->dev, "failed to request gpio %d: %d\n",
> - lcd->im_pins[i], ret);
> - return -EINVAL;
> - }
> - }
> + } else
> + lcd->use_im_pins = 0;
>
> lcdev = lcd_device_register("mxsfb", &spi->dev, lcd, &hx8357_ops);
> if (IS_ERR(lcdev)) {
> --
> 1.8.1.2
>
^ permalink raw reply
* Re: [PATCH 2/2] fb: backlight: HX8357: Add HX8369 support
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-24 14:31 UTC (permalink / raw)
To: Alexandre Belloni
Cc: linux-fbdev, jimwall, brian, Maxime Ripard, linux-kernel,
Richard Purdie, Florian Tobias Schandinat
In-Reply-To: <1371839259-543-2-git-send-email-alexandre.belloni@free-electrons.com>
On 20:27 Fri 21 Jun , Alexandre Belloni wrote:
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/video/backlight/hx8357.c | 183 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 175 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
> index 69f5672..b1c4042 100644
> --- a/drivers/video/backlight/hx8357.c
> +++ b/drivers/video/backlight/hx8357.c
> @@ -71,6 +71,18 @@
> #define HX8357_SET_POWER_NORMAL 0xd2
> #define HX8357_SET_PANEL_RELATED 0xe9
>
> +#define HX8369_SET_DISPLAY_BRIGHTNESS 0x51
> +#define HX8369_WRITE_CABC_DISPLAY_VALUE 0x53
> +#define HX8369_WRITE_CABC_BRIGHT_CTRL 0x55
> +#define HX8369_WRITE_CABC_MIN_BRIGHTNESS 0x5e
> +#define HX8369_SET_POWER 0xb1
> +#define HX8369_SET_DISPLAY_MODE 0xb2
> +#define HX8369_SET_DISPLAY_WAVEFORM_CYC 0xb4
> +#define HX8369_SET_VCOM 0xb6
> +#define HX8369_SET_EXTENSION_COMMAND 0xb9
> +#define HX8369_SET_GIP 0xd5
> +#define HX8369_SET_GAMMA_CURVE_RELATED 0xe0
> +
> struct hx8357_data {
> unsigned im_pins[HX8357_NUM_IM_PINS];
> unsigned reset;
> @@ -144,6 +156,61 @@ static u8 hx8357_seq_display_mode[] = {
> HX8357_SET_DISPLAY_MODE_RGB_INTERFACE,
> };
>
> +static u8 hx8369_seq_write_CABC_min_brightness[] = {
> + HX8369_WRITE_CABC_MIN_BRIGHTNESS, 0x00,
> +};
> +
> +static u8 hx8369_seq_write_CABC_control[] = {
> + HX8369_WRITE_CABC_DISPLAY_VALUE, 0x24,
> +};
> +
> +static u8 hx8369_seq_set_display_brightness[] = {
> + HX8369_SET_DISPLAY_BRIGHTNESS, 0xFF,
> +};
> +
> +static u8 hx8369_seq_write_CABC_control_setting[] = {
> + HX8369_WRITE_CABC_BRIGHT_CTRL, 0x02,
> +};
> +
> +static u8 hx8369_seq_extension_command[] = {
> + HX8369_SET_EXTENSION_COMMAND, 0xff, 0x83, 0x69,
> +};
> +
> +static u8 hx8369_seq_display_related[] = {
> + HX8369_SET_DISPLAY_MODE, 0x00, 0x2b, 0x03, 0x03, 0x70, 0x00,
> + 0xff, 0x00, 0x00, 0x00, 0x00, 0x03, 0x03, 0x00, 0x01,
> +};
> +
> +static u8 hx8369_seq_panel_waveform_cycle[] = {
> + HX8369_SET_DISPLAY_WAVEFORM_CYC, 0x0a, 0x1d, 0x80, 0x06, 0x02,
> +};
> +
> +static u8 hx8369_seq_set_address_mode[] = {
> + HX8357_SET_ADDRESS_MODE, 0x00,
> +};
> +
> +static u8 hx8369_seq_vcom[] = {
> + HX8369_SET_VCOM, 0x3e, 0x3e,
> +};
> +
> +static u8 hx8369_seq_gip[] = {
> + HX8369_SET_GIP, 0x00, 0x01, 0x03, 0x25, 0x01, 0x02, 0x28, 0x70,
> + 0x11, 0x13, 0x00, 0x00, 0x40, 0x26, 0x51, 0x37, 0x00, 0x00, 0x71,
> + 0x35, 0x60, 0x24, 0x07, 0x0f, 0x04, 0x04,
> +};
> +
> +static u8 hx8369_seq_power[] = {
> + HX8369_SET_POWER, 0x01, 0x00, 0x34, 0x03, 0x00, 0x11, 0x11, 0x32,
> + 0x2f, 0x3f, 0x3f, 0x01, 0x3a, 0x01, 0xe6, 0xe6, 0xe6, 0xe6, 0xe6,
> +};
> +
> +static u8 hx8369_seq_gamma_curve_related[] = {
> + HX8369_SET_GAMMA_CURVE_RELATED, 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d,
> + 0x2e, 0x4a, 0x08, 0x0e, 0x0f, 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
> + 0x00, 0x0d, 0x19, 0x2f, 0x3b, 0x3d, 0x2e, 0x4a, 0x08, 0x0e, 0x0f,
> + 0x14, 0x16, 0x14, 0x14, 0x14, 0x1e,
> +};
> +
> static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
> u8 *txbuf, u16 txlen,
> u8 *rxbuf, u16 rxlen)
> @@ -242,6 +309,16 @@ static int hx8357_exit_standby(struct lcd_device *lcdev)
> return 0;
> }
>
> +static void hx8357_lcd_reset(struct hx8357_data *lcd)
> +{
> + gpio_set_value(lcd->reset, 1);
> + usleep_range(10000, 12000);
> + gpio_set_value(lcd->reset, 0);
> + usleep_range(10000, 12000);
> + gpio_set_value(lcd->reset, 1);
> + msleep(120);
> +}
> +
> static int hx8357_lcd_init(struct lcd_device *lcdev)
> {
> struct hx8357_data *lcd = lcd_get_data(lcdev);
> @@ -257,13 +334,7 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
> gpio_set_value_cansleep(lcd->im_pins[2], 1);
> }
>
> - /* Reset the screen */
> - gpio_set_value(lcd->reset, 1);
> - usleep_range(10000, 12000);
> - gpio_set_value(lcd->reset, 0);
> - usleep_range(10000, 12000);
> - gpio_set_value(lcd->reset, 1);
> - msleep(120);
> + hx8357_lcd_reset(lcd);
>
> ret = hx8357_spi_write_array(lcdev, hx8357_seq_power,
> ARRAY_SIZE(hx8357_seq_power));
> @@ -359,6 +430,97 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
> return 0;
> }
>
> +static int hx8369_lcd_init(struct lcd_device *lcdev)
> +{
> + struct hx8357_data *lcd = lcd_get_data(lcdev);
> + int ret;
> +
> + hx8357_lcd_reset(lcd);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_extension_command,
> + ARRAY_SIZE(hx8369_seq_extension_command));
> + if (ret < 0)
> + return ret;
> + usleep_range(10000, 12000);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_display_related,
> + ARRAY_SIZE(hx8369_seq_display_related));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_panel_waveform_cycle,
> + ARRAY_SIZE(hx8369_seq_panel_waveform_cycle));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_address_mode,
> + ARRAY_SIZE(hx8369_seq_set_address_mode));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_vcom,
> + ARRAY_SIZE(hx8369_seq_vcom));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_gip,
> + ARRAY_SIZE(hx8369_seq_gip));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_power,
> + ARRAY_SIZE(hx8369_seq_power));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
> + if (ret < 0)
> + return ret;
> +
> + msleep(120);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_gamma_curve_related,
> + ARRAY_SIZE(hx8369_seq_gamma_curve_related));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_byte(lcdev, HX8357_EXIT_SLEEP_MODE);
> + if (ret < 0)
> + return ret;
> + usleep_range(1000, 1200);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_write_CABC_control,
> + ARRAY_SIZE(hx8369_seq_write_CABC_control));
> + if (ret < 0)
> + return ret;
> + usleep_range(10000, 12000);
> +
> + ret = hx8357_spi_write_array(lcdev,
> + hx8369_seq_write_CABC_control_setting,
> + ARRAY_SIZE(hx8369_seq_write_CABC_control_setting));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_array(lcdev,
> + hx8369_seq_write_CABC_min_brightness,
> + ARRAY_SIZE(hx8369_seq_write_CABC_min_brightness));
> + if (ret < 0)
> + return ret;
> + usleep_range(10000, 12000);
> +
> + ret = hx8357_spi_write_array(lcdev, hx8369_seq_set_display_brightness,
> + ARRAY_SIZE(hx8369_seq_set_display_brightness));
> + if (ret < 0)
> + return ret;
> +
> + ret = hx8357_spi_write_byte(lcdev, HX8357_SET_DISPLAY_ON);
> + if (ret < 0)
> + return ret;
> + msleep(100);
> +
> + return 0;
> +}
> +
> #define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
>
> static int hx8357_set_power(struct lcd_device *lcdev, int power)
> @@ -461,7 +623,11 @@ static int hx8357_probe(struct spi_device *spi)
> }
> spi_set_drvdata(spi, lcdev);
>
> - ret = hx8357_lcd_init(lcdev);
> + ret = -ENODEV;
> + if (of_device_is_compatible(spi->dev.of_node, "himax,hx8357"))
> + ret = hx8357_lcd_init(lcdev);
> + else if (of_device_is_compatible(spi->dev.of_node, "himax,hx8369"))
> + ret = hx8369_lcd_init(lcdev);
so use the data from the compatible to get the function pointer
and does not have to do a list of if/else compatible
> if (ret) {
> dev_err(&spi->dev, "Couldn't initialize panel\n");
> goto init_error;
> @@ -486,6 +652,7 @@ static int hx8357_remove(struct spi_device *spi)
>
> static const struct of_device_id hx8357_dt_ids[] = {
> { .compatible = "himax,hx8357" },
> + { .compatible = "himax,hx8369" },
> {},
> };
> MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
> --
> 1.8.1.2
>
^ permalink raw reply
* Re: [PATCH 2/2] fb: fix atyfb unused data warnings
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-06-24 14:37 UTC (permalink / raw)
To: Randy Dunlap
Cc: LKML, Linux Fbdev development list, Andrew Morton, Paul Mackerras,
Benjamin Herrenschmidt, Tomi Valkeinen
In-Reply-To: <51C26B56.20802@infradead.org>
On 19:39 Wed 19 Jun , Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix compiler warnings of data defined but not used. They are only used
> with certain kconfig settings.
>
> drivers/video/aty/atyfb_base.c:534:13: warning: 'ram_dram' defined but not used [-Wunused-variable]
> drivers/video/aty/atyfb_base.c:535:13: warning: 'ram_resv' defined but not used [-Wunused-variable]
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/aty/atyfb_base.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- linux-next-20130619.orig/drivers/video/aty/atyfb_base.c
> +++ linux-next-20130619/drivers/video/aty/atyfb_base.c
> @@ -531,8 +531,10 @@ static int correct_chipset(struct atyfb_
> return 0;
> }
>
> +#if defined(CONFIG_FB_ATY_GX) || defined(CONFIG_FB_ATY_CT)
> static char ram_dram[] = "DRAM";
> static char ram_resv[] = "RESV";
use __maybe_unused macro instead of the ifdef
> +#endif
> #ifdef CONFIG_FB_ATY_GX
> static char ram_vram[] = "VRAM";
> #endif /* CONFIG_FB_ATY_GX */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [V2 1/7] video: mmp: rb swap setting update for new LCD driver
From: Daniel Drake @ 2013-06-24 14:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACDDiy8=WRmnGMFr55uVWUE6+NNy3pMCeBfV8E9urAeW6VVEMw@mail.gmail.com>
On Mon, Jun 24, 2013 at 2:45 AM, jett zhou <jett.zhou@gmail.com> wrote:
> Sorry that the comments of the patch is not so clear.
> As you might know,We can set two rbswap setting, one is based on
> pix_fmt to set into DMA input part, the other is to configured in the output
> interface part.
> This patch include below change and enhancement:
> 1) The input format which support rbswap is based RGB format, YUV
> series did not support now. So you can see the patch will change the rbswap
> value based on specific RGB format,eg. RGB will set rbswap = 1, BGR will not
> set it.
What if I'm working with a display that doesn't need or want RB
swapping? Lets say I am working with format PIXFMT_RGB565, and running
your patch. dmafetch_set_fmt() gets called, and fmt_to_reg() sets
rbswap to 1.
This means that dmafetch_set_fmt() writes a '1' into the appropriate
RB-swapping bit in the LCD_PN_CTRL0 register, and this triggers the
"DMA input" swapping that you mentioned. But I never asked for RB
swapping...
> This part setting is controlled in driver internally, will not
> depend on platform configure(link_config) any more.
Your patch seems to depend very clearly on link_config.
> 2) The output part of rbswap is depend on link_config which is
> defined in specific platfrom.
> It will setting bit27:24 of 0x01DC register, eg. DSI output
> interface can be controlled by this.
I don't think you should add more disorganised/undocumented (ab)use of
this magic link_config variable. You should create new, dedicated,
documented fields.
Your comment above suggests that this RB-swapping behaviour is
something that is imposed by the output device. In which case, this
should be a configuration parameter on the panel, not on the path
structure.
> TTC_dkb does not support dsi, the link_config is no used anymore.
Then you should fix up ttc_dkb before submitting this patch.
Thanks
Daniel
^ permalink raw reply
* Re: MMP display subsystem questions
From: Daniel Drake @ 2013-06-24 14:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51C7F4C5.9040903@marvell.com>
On Mon, Jun 24, 2013 at 1:27 AM, Zhou Zhu <zzhu3@marvell.com> wrote:
> Actually besides fb, v4l2 and an IOCTL based interface to support what
> fb/v4l2 not provided are also added. The v4l2 or private interface could
> also directly use same interface defined in include/video/mmp_disp.h to get
> path/overlay and manipulate the hardware.
> Patches for these interfaces would be submitted later and documentation
> would also be added.
This kind of reasoning normally does not act as justification for
adding private interfaces to the kernel. I doubt such patches would be
accepted. Instead, you would be encouraged to help improve the
standard interfaces that are missing the functionality you require.
> These settings are software related rather than what hw supports.
> Vsync settings include whether fb wait vsync interrupt to sync buffers when
> pan display or whether send uevent in vsync (which is a mechanism for
> Android).
> For yres_virtual size which is related with pre-reserved fb buffer size and
> some buffer sync mechanism.
> As these settings are pure fb related so we just not place it in panel.
Thanks, that is clear now.
>> 2. What about pix_fmt_out? Why is pixel format dumped into the mode
>> specifier? If there is a supported panel that really can support
>> different pixel formats, I would expect it to support all the pixel
>> formats for all the resolution. This also seems unused within the
>> driver.
>
> We added it due to 2 reasons:
> 1. It impacts output timing settings inside controller for dsi case (we have
> already in-home patches and would be submitted later). So it's required to
> indicate that timing need to be changed.
> 2. It's still possible that for some panel that could support different
> resolution and pixel formats, but it might not support all pixel formats for
> all resolutions.
>
> Also this variable is not used for dumb panel as it's already covered in
> dumb mode in link config - dumb mode includes not only 16bpp or 24bpp but
> also whether low 16 bits or high 16 bits used in 24 bits' lines. For coming
> DSI panel, it's required and would be used.
It is a little irritating trying to work with this driver when a
number of undocumented strange details are dependent on unsubmitted
work.
I would suggest keeping this simple and making the pixel format
specific to the panel, not specific to the resolution. While maybe it
is possible we will see a panel that supports certain pixel formats at
only certain resolutions, that seems unlikely, and doesn't seem to be
anything we are working with at the moment?
Moving pixel format out of the mode definition will help with
compatibility with standard devicetree display-timings bindings, which
is the reason that I asked this question.
Daniel
^ permalink raw reply
* Re: [V2 5/7] video: mmp: add pitch info in mmp_win structure
From: Daniel Drake @ 2013-06-24 16:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACDDiy9CP2NyLAwCSr97yOAS50gz6HumY9cAHwO1DFRSCy9L-w@mail.gmail.com>
On Mon, Jun 24, 2013 at 4:34 AM, jett zhou <jett.zhou@gmail.com> wrote:
> pitch is used to represent line length in byte, the usage depends
> on pix_fmt.
> If the fmt is YUV , the pitch[0] will be Y length, pitch[1] will
> be U length, pitch[2] will be V lenth.
> If the fmt is RGB, the picth[0] will be line lenth, and
> pitch[1]/pitch[2] will be 0 and not be used.
This description is clear, thanks - hopefully you can write it with
such clarity in the comment :)
> For the patch rolling, do you mean combine the patch5 and patch6
> by one patch?
I view patch 6 as a cleanup (consolidating and removing duplication of
code), so I would leave that one separate. Patch 6 should not interact
with any pitch[] stuff.
Then you can write a followup patch which adds the pitch[] header,
*and* modifies mmpfb_set_par() to write to pitch[], *and* acts upon
pitch[] in dmafetch_set_fmt (patch 7). This way, the pitch variable is
defined, documented, written to, and acted upon all in the same patch,
the meaning will then be very clear.
Thanks
Daniel
^ 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