* Re: [PATCH 0/5] at91: atmel_lcdfb: regression fixes and cpu_is removal
From: Johan Hovold @ 2013-02-10 18:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130210004740.GH16278@quad.lixom.net>
On Sun, Feb 10, 2013 at 1:47 AM, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Feb 08, 2013 at 05:35:13PM +0100, Nicolas Ferre wrote:
>> These patches fix a regression in 16-bpp support for older SOCs which
>> use IBGR:555 rather than BGR:565 pixel layout. Use SOC-type to
>> determine if the controller uses the intensity-bit and restore the
>> old layout in that case.
>>
>> The last patch is a removal of uses of cpu_is_xxxx() macros in
>> atmel_lcdfb with a platform-device-id table and static
>> configurations.
>>
>>
>> Patches from Johan Hovold taken from: "[PATCH 0/3] atmel_lcdfb: fix
>> 16-bpp regression" and "[PATCH v2 0/3] ARM: at91/avr32/atmel_lcdfb:
>> remove cpu_is macros" patch series to form a clean patch series with
>> my signature.
>>
>> Arnd, Olof, as it seems that old fbdev drivers are not so much
>> reviewed those days, can we take the decision to queue this material
>> through arm-soc with other AT91 drivers updates?
>
> It would be beneficial to get an ack from Florian. Was he involved in
> the review of the code that regressed 16-bpp support in the first
> place? When was the regression introduced?
In v3.4 by commit 787f9fd2328 ("atmel_lcdfb: support 16bit BGR:565 mode,
remove unsupported 15bit modes").
Johan
^ permalink raw reply
* [PATCH] video/modedb: fb_find_nearest_mode: take vmode into account
From: Floris Bos @ 2013-02-10 20:23 UTC (permalink / raw)
To: linux-fbdev
Previously fb_find_nearest_mode() only searched the modelist for a video mode that best matches the
desired resolution and refresh rate.
With this patch it also takes the vmode into account if there is more then one mode with the same
resolution and refresh rate.
Typical use case is HDMI TVs that support both 1080p60 and 1080i60
Signed-off-by: Floris Bos <bos@je-eigen-domein.nl>
---
drivers/video/modedb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index a9a907c..e852371 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c
@@ -913,6 +913,8 @@ const struct fb_videomode *fb_find_best_mode(const struct fb_var_screeninfo *var
* Finds best matching videomode, smaller or greater in dimension.
* If more than 1 videomode is found, will return the videomode with
* the closest refresh rate.
+ * If multiple modes with the same resolution and refresh rate are found
+ * pick the one with the matching vmode (e.g. non-interlaced)
*/
const struct fb_videomode *fb_find_nearest_mode(const struct fb_videomode *mode,
struct list_head *head)
@@ -939,6 +941,8 @@ const struct fb_videomode *fb_find_nearest_mode(const struct fb_videomode *mode,
if (diff_refresh > d) {
diff_refresh = d;
best = cmode;
+ } else if (diff_refresh = d && cmode->vmode = mode->vmode) {
+ best = cmode;
}
}
}
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v2 1/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Andi Shyti @ 2013-02-10 23:51 UTC (permalink / raw)
To: Ruslan Bilovol
Cc: andi, tomi.valkeinen, FlorianSchandinat, linux-fbdev,
linux-kernel, linux-omap
In-Reply-To: <1360338220-12753-2-git-send-email-ruslan.bilovol@ti.com>
Hi Ruslan,
> TC358765 is DSI-to-LVDS transmitter from Toshiba, used in
> OMAP44XX Blaze Tablet and Blaze Tablet2 boards.
I think it's fine, just some nitpicks and checkpatch warnings
> +struct {
> + struct device *dev;
> + struct dentry *dir;
> +} tc358765_debug;
Should this be static?
> +struct tc358765_reg {
> + const char *name;
> + u16 reg;
> + u8 perm:2;
> +} tc358765_regs[] = {
Should this be static as well?
> + { "D1S_ZERO", D1S_ZERO, A_RW },
> + { "D2S_ZERO", D2S_ZERO, A_RW },
> + { "D3S_ZERO", D3S_ZERO, A_RW },
> + { "PPI_CLRFLG", PPI_CLRFLG, A_RW },
> + { "PPI_CLRSIPO", PPI_CLRSIPO, A_RW },
> + { "PPI_HSTimeout", PPI_HSTimeout, A_RW },
WARNING: Avoid CamelCase: <PPI_HSTimeout>
#136: FILE: video/omap2/displays/panel-tc358765.c:136:
+ { "PPI_HSTimeout", PPI_HSTimeout, A_RW },
> + { "PPI_HSTimeoutEnable", PPI_HSTimeoutEnable, A_RW },
WARNING: Avoid CamelCase: <PPI_HSTimeoutEnable>
#137: FILE: video/omap2/displays/panel-tc358765.c:137:
+ { "PPI_HSTimeoutEnable", PPI_HSTimeoutEnable, A_RW },
> +static int tc358765_read_block(u16 reg, u8 *data, int len)
> +{
> + unsigned char wb[2];
> + struct i2c_msg msg[2];
> + int r;
> + mutex_lock(&tc358765_i2c->xfer_lock);
> + wb[0] = (reg & 0xff00) >> 8;
> + wb[1] = reg & 0xff;
> + msg[0].addr = tc358765_i2c->client->addr;
> + msg[0].len = 2;
> + msg[0].flags = 0;
> + msg[0].buf = wb;
> + msg[1].addr = tc358765_i2c->client->addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = len;
> + msg[1].buf = data;
> +
> + r = i2c_transfer(tc358765_i2c->client->adapter, msg, ARRAY_SIZE(msg));
> + mutex_unlock(&tc358765_i2c->xfer_lock);
> +
> + if (r = ARRAY_SIZE(msg))
> + return len;
> +
> + return r;
If you like, here you could do
return (r = ARRAY_SIZE(msg)) ? len : r;
Usually I like more this notation because keeps the code more
compact and immediate to read, but you don't have to
> + mutex_lock(&tc358765_i2c->xfer_lock);
> + ret = i2c_transfer(tc358765_i2c->client->adapter, &msg, 1);
> + mutex_unlock(&tc358765_i2c->xfer_lock);
> +
> + if (ret != 1)
> + return ret;
> + return 0;
Also here you could do
return (ret != 1) ? ret : 0;
But this is more taste :)
> +static ssize_t tc358765_seq_write(struct file *filp, const char __user *ubuf,
> + size_t size, loff_t *ppos)
> +{
> + struct device *dev = tc358765_debug.dev;
> + unsigned i, reg_count;
> + u32 value = 0;
> + int error = 0;
> + /* kids, don't use register names that long */
I won't, promised, but please, drop this comment :)
> + char name[30];
> + char buf[50];
> +
> + if (size >= sizeof(buf))
> + size = sizeof(buf);
what's the point of this?
> +static void tc358765_uninitialize_debugfs(void)
> +{
> + if (tc358765_debug.dir)
> + debugfs_remove_recursive(tc358765_debug.dir);
WARNING: debugfs_remove_recursive(NULL) is safe this check is probably not required
#435: FILE: video/omap2/displays/panel-tc358765.c:435:
+ if (tc358765_debug.dir)
+ debugfs_remove_recursive(tc358765_debug.dir);
> +static struct tc358765_board_data *get_board_data(struct omap_dss_device
> + *dssdev)
> +{
> + return (struct tc358765_board_data *)dssdev->data;
You shouldn't need for this cast, does it complain?
> +}
> +
> +static void tc358765_get_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + *timings = dssdev->panel.timings;
> +}
> +
> +static void tc358765_set_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + dev_info(&dssdev->dev, "set_timings() not implemented\n");
... then drop the function :)
> + if ((pins[2] & 1) || (pins[3] & 1)) {
> + lanes |= (1 << 1);
> + ret |= tc358765_write_register(dssdev, PPI_D0S_CLRSIPOCOUNT,
> + board_data->clrsipo);
> + }
> + if ((pins[4] & 1) || (pins[5] & 1)) {
> + lanes |= (1 << 2);
> + ret |= tc358765_write_register(dssdev, PPI_D1S_CLRSIPOCOUNT,
> + board_data->clrsipo);
> + }
> + if ((pins[6] & 1) || (pins[7] & 1)) {
> + lanes |= (1 << 3);
> + ret |= tc358765_write_register(dssdev, PPI_D2S_CLRSIPOCOUNT,
> + board_data->clrsipo);
> + }
> + if ((pins[8] & 1) || (pins[9] & 1)) {
> + lanes |= (1 << 4);
> + ret |= tc358765_write_register(dssdev, PPI_D3S_CLRSIPOCOUNT,
> + board_data->clrsipo);
> + }
Can't this be done in one single multiwrighting command since
this registers are consecutive?
You build once the array to write and you send it at once.
Moreover it would be nice to have a name for all those nombers
> + ret |= tc358765_write_register(dssdev, HTIM1,
> + (tc358765_timings.hbp << 16) | tc358765_timings.hsw);
> + ret |= tc358765_write_register(dssdev, HTIM2,
> + ((tc358765_timings.hfp << 16) | tc358765_timings.x_res));
> + ret |= tc358765_write_register(dssdev, VTIM1,
> + ((tc358765_timings.vbp << 16) | tc358765_timings.vsw));
> + ret |= tc358765_write_register(dssdev, VTIM2,
> + ((tc358765_timings.vfp << 16) | tc358765_timings.y_res));
also this and all the other cases I haven't checked
> +static int tc358765_enable(struct omap_dss_device *dssdev)
> +{
> + struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
> + int r = 0;
this initialisation is useless
> + if (r) {
> + dev_dbg(&dssdev->dev, "enable failed\n");
Here you could choose a different print level, I would have used
dev_err instead.
> +static int tc358765_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + tc358765_i2c = devm_kzalloc(&client->dev, sizeof(*tc358765_i2c), GFP_KERNEL);
WARNING: line over 80 characters
#927: FILE: video/omap2/displays/panel-tc358765.c:927:
+ tc358765_i2c = devm_kzalloc(&client->dev, sizeof(*tc358765_i2c), GFP_KERNEL);
> + /* store i2c_client pointer on private data structure */
> + tc358765_i2c->client = client;
> +
> + /* store private data structure pointer on i2c_client structure */
> + i2c_set_clientdata(client, tc358765_i2c);
> +
> + /* init mutex */
> + mutex_init(&tc358765_i2c->xfer_lock);
> + dev_err(&client->dev, "D2L i2c initialized\n");
while here dev_dbg (or dev_info) are better.
> +static int __init tc358765_init(void)
> +{
> + int r;
> + tc358765_i2c = NULL;
> + r = i2c_add_driver(&tc358765_i2c_driver);
> + if (r < 0) {
> + printk(KERN_WARNING "d2l i2c driver registration failed\n");
WARNING: Prefer netdev_warn(netdev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
#981: FILE: video/omap2/displays/panel-tc358765.c:981:
+ printk(KERN_WARNING "d2l i2c driver registration failed\n");
Andi
^ permalink raw reply
* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Tomi Valkeinen @ 2013-02-11 8:21 UTC (permalink / raw)
To: Marcus Lorentzon
Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
m.szyprowski@samsung.com, Daniel Vetter, Vikas Sajjan,
inki.dae@samsung.com, dh09.lee@samsung.com,
ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <511511B3.3060404@stericsson.com>
[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]
On 2013-02-08 16:54, Marcus Lorentzon wrote:
> On 02/08/2013 03:02 PM, Tomi Valkeinen wrote:
>> On 2013-02-08 15:28, Marcus Lorentzon wrote:
>>
>>> When we do that we stop->setup->start during blanking. So our "DSS" is
>>> optimized to be able to do that without getting blocked. All DSI video
>>> mode panels (and DPI) products we have done so far have not had any
>>> issue with that (as long as DSI HS clock is set to continuous). I think
>>> this approach is less platform dependant, as long as there is no SoC
>>> that take more than a blanking period to reconfigure.
>> So do you stop, setup and start the link with CPU, and this has to be
>> happen during blanking? Isn't that prone to errors? Or did you mean that
>> the hardware handles that automatically?
>>
>> In OMAP DSS there are so called shadow registers, that can be programmed
>> at any time. The you set a bit (GO bit), which tells the hardware to
>> take the new settings into use at the next vblank.
>>
>> From DSI driver's perspective the link is never stopped when
>> reconfiguring the video timings. However, many other settings have to be
>> configured when the link is disabled.
>
> Yeah, you lucky guys with the GO bit ;). No, we actually do CPU
> stop,setup,start. But since it is video mode, master is driving the sync
> so it is not a hard deadline. It is enough to restart before pixels
> start to degrade. On an LCD that is not so much time, but on an OLED it
> could be 10 secs :). Anyway, we have had several mass products with this
> soft solution and it has worked well.
Ah, ok. But in that case what you said in an earlier mail is not quite
correct: "I think this approach is less platform dependant, as long as
there is no SoC that take more than a blanking period to reconfigure.".
So in your approach the reconfiguration doesn't have to be done inside
the blanking period, but before the panel's picture starts to fade?
I don't know... It's early Monday morning, and not enough coffee yet,
but I get a bit uneasy feeling if I think that your method of
reconfiguring would be the only think allowed by the CDF API.
Some SoCs do support reconfiguring on the fly, without disabling the
link. It would not be nice if we didn't allow this to happen. And
actually, we're not only talking about SoCs here, but also any display
devices, like external buffer chips etc. They may also offer means to
change configs on the fly.
Well, I don't have any hard point about this at the moment, but I think
we should think different approaches how the configuration can be done.
> I understand, but removing the omap prefix doesn't mean it has to go in
> the panel slave port/bus settings. I still can't see why this should be
> configuration on the panel driver and not the DSI master driver. Number
> of pins might be useful since you might start with one lane and then
> activate the rest. But partial muxing (pre pinmux) doesn't seem to be
> something the panel should control or know anything about. Sounds like
> normal platform/DT data per product/board.
I think one case where this kind of pin configuration is needed, and
which also dictates that all panel related configuration has to be in
the panel's data, not in the DSI master's data, is hotplug.
If you have a board that has two panels connected to the same video
output, probably via some kind of mux, at least for the sensitive pins
like DSI, only one of the panels can be enabled at a time. The panels
can have different wiring, and thus the panel driver would need to
configure everything related to the bus when it's starting up.
The same also happens if you have a true hotplug, i.e. you can remove
the panel totally and plug in a new one. Again the wiring can be
different, and needs to be set up.
And, as I said, this means that all relevant data about the video bus
has to be in the panel's data, so that each panel can have its own set
of, say, pin configuration.
Hotplug is not a use case we should aim to support for the CDF v1, but I
think we should strive not to prevent hotplug either. So if we can
design the API so that hotplug is possible, I say let's do that.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] [PATCH] video/modedb: fb_find_nearest_mode: take vmode into account
From: Hans de Goede @ 2013-02-11 9:28 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360527814-28883-1-git-send-email-bos@je-eigen-domein.nl>
Hi,
On 02/10/2013 09:23 PM, Floris Bos wrote:
> Previously fb_find_nearest_mode() only searched the modelist for a video mode that best matches the
> desired resolution and refresh rate.
> With this patch it also takes the vmode into account if there is more then one mode with the same
> resolution and refresh rate.
> Typical use case is HDMI TVs that support both 1080p60 and 1080i60
>
> Signed-off-by: Floris Bos <bos@je-eigen-domein.nl>
> ---
> drivers/video/modedb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
> index a9a907c..e852371 100644
> --- a/drivers/video/modedb.c
> +++ b/drivers/video/modedb.c
> @@ -913,6 +913,8 @@ const struct fb_videomode *fb_find_best_mode(const struct fb_var_screeninfo *var
> * Finds best matching videomode, smaller or greater in dimension.
> * If more than 1 videomode is found, will return the videomode with
> * the closest refresh rate.
> + * If multiple modes with the same resolution and refresh rate are found
> + * pick the one with the matching vmode (e.g. non-interlaced)
> */
> const struct fb_videomode *fb_find_nearest_mode(const struct fb_videomode *mode,
> struct list_head *head)
> @@ -939,6 +941,8 @@ const struct fb_videomode *fb_find_nearest_mode(const struct fb_videomode *mode,
> if (diff_refresh > d) {
> diff_refresh = d;
> best = cmode;
> + } else if (diff_refresh = d && cmode->vmode = mode->vmode) {
> + best = cmode;
> }
> }
> }
>
Hmm, my version of this patch was more conservative, only comparing the INTERLACED bit
of vmode. I assume you've tested this, and it works as advertised? If so ack.
Regards,
Hans
^ permalink raw reply
* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Marcus Lorentzon @ 2013-02-11 9:31 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
m.szyprowski@samsung.com, Daniel Vetter, Vikas Sajjan,
inki.dae@samsung.com, dh09.lee@samsung.com,
ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <5118AA00.2000505@ti.com>
On 02/11/2013 09:21 AM, Tomi Valkeinen wrote:
> On 2013-02-08 16:54, Marcus Lorentzon wrote:
>> On 02/08/2013 03:02 PM, Tomi Valkeinen wrote:
>>> On 2013-02-08 15:28, Marcus Lorentzon wrote:
>>>
>>>> When we do that we stop->setup->start during blanking. So our "DSS" is
>>>> optimized to be able to do that without getting blocked. All DSI video
>>>> mode panels (and DPI) products we have done so far have not had any
>>>> issue with that (as long as DSI HS clock is set to continuous). I think
>>>> this approach is less platform dependant, as long as there is no SoC
>>>> that take more than a blanking period to reconfigure.
>>> So do you stop, setup and start the link with CPU, and this has to be
>>> happen during blanking? Isn't that prone to errors? Or did you mean that
>>> the hardware handles that automatically?
>>>
>>> In OMAP DSS there are so called shadow registers, that can be programmed
>>> at any time. The you set a bit (GO bit), which tells the hardware to
>>> take the new settings into use at the next vblank.
>>>
>>> From DSI driver's perspective the link is never stopped when
>>> reconfiguring the video timings. However, many other settings have to be
>>> configured when the link is disabled.
>> Yeah, you lucky guys with the GO bit ;). No, we actually do CPU
>> stop,setup,start. But since it is video mode, master is driving the sync
>> so it is not a hard deadline. It is enough to restart before pixels
>> start to degrade. On an LCD that is not so much time, but on an OLED it
>> could be 10 secs :). Anyway, we have had several mass products with this
>> soft solution and it has worked well.
> Ah, ok. But in that case what you said in an earlier mail is not quite
> correct: "I think this approach is less platform dependant, as long as
> there is no SoC that take more than a blanking period to reconfigure.".
> So in your approach the reconfiguration doesn't have to be done inside
> the blanking period, but before the panel's picture starts to fade?
>
> I don't know... It's early Monday morning, and not enough coffee yet,
> but I get a bit uneasy feeling if I think that your method of
> reconfiguring would be the only think allowed by the CDF API.
>
> Some SoCs do support reconfiguring on the fly, without disabling the
> link. It would not be nice if we didn't allow this to happen. And
> actually, we're not only talking about SoCs here, but also any display
> devices, like external buffer chips etc. They may also offer means to
> change configs on the fly.
>
> Well, I don't have any hard point about this at the moment, but I think
> we should think different approaches how the configuration can be done.
Ok, so what about a compromise which I think would work for "both" HWs
... we allow the "configure" operation during video on, then each HW
driver could decide if this means you have to stop or not to do the
changes required. But then it is also important that we keep all config
in one struct and not split it up. Otherwise HW like ours that has so be
stopped could need to stop once for each setting/operation called.
So in short, allow configure(bus_params) during video on, keep all
bus_params in the struct. It is in kernel struct so it can easily be
changed/refactored.
>
>> I understand, but removing the omap prefix doesn't mean it has to go in
>> the panel slave port/bus settings. I still can't see why this should be
>> configuration on the panel driver and not the DSI master driver. Number
>> of pins might be useful since you might start with one lane and then
>> activate the rest. But partial muxing (pre pinmux) doesn't seem to be
>> something the panel should control or know anything about. Sounds like
>> normal platform/DT data per product/board.
> I think one case where this kind of pin configuration is needed, and
> which also dictates that all panel related configuration has to be in
> the panel's data, not in the DSI master's data, is hotplug.
>
> If you have a board that has two panels connected to the same video
> output, probably via some kind of mux, at least for the sensitive pins
> like DSI, only one of the panels can be enabled at a time. The panels
> can have different wiring, and thus the panel driver would need to
> configure everything related to the bus when it's starting up.
>
> The same also happens if you have a true hotplug, i.e. you can remove
> the panel totally and plug in a new one. Again the wiring can be
> different, and needs to be set up.
>
> And, as I said, this means that all relevant data about the video bus
> has to be in the panel's data, so that each panel can have its own set
> of, say, pin configuration.
>
> Hotplug is not a use case we should aim to support for the CDF v1, but I
> think we should strive not to prevent hotplug either. So if we can
> design the API so that hotplug is possible, I say let's do that.
>
Again, this probing and bus muxing is platform/bus specific and not
panel specific. Any panel of that type will only ever work on Omap (or
someone else implementing the same muxing features) as I see it. So why
not just put that config on the bus master, dispc? I still can't see how
this is panel config. You are only pushing CDF API and meta data to
describe something that is only needed by one bus master. I have never
seen any DSI slave that can change their pin config. And since there is
no generic hot plug detect of DSI panels, at least not before DSI bus is
available, I have to assume this probing it very platform specific. We
have some products that provide 1-2 gpios to specify what panel is
available, some use I2C sensor probing to then assume the panel plugged.
At least in the first step I don't think this type of hot plug should be
in the API. Then once the base panel driver is in we could discuss
different solutions for you hot plug scenario.
/BR
/Marcus
^ permalink raw reply
* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Tomi Valkeinen @ 2013-02-11 10:14 UTC (permalink / raw)
To: Marcus Lorentzon
Cc: Tomasz Figa, Laurent Pinchart, Tomasz Figa,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-samsung-soc@vger.kernel.org, kyungmin.park@samsung.com,
m.szyprowski@samsung.com, Daniel Vetter, Vikas Sajjan,
inki.dae@samsung.com, dh09.lee@samsung.com,
ville.syrjala@intel.com, s.nawrocki@samsung.com
In-Reply-To: <5118BA80.1020604@stericsson.com>
[-- Attachment #1: Type: text/plain, Size: 4844 bytes --]
On 2013-02-11 11:31, Marcus Lorentzon wrote:
> Ok, so what about a compromise which I think would work for "both" HWs
> ... we allow the "configure" operation during video on, then each HW
> driver could decide if this means you have to stop or not to do the
> changes required. But then it is also important that we keep all config
> in one struct and not split it up. Otherwise HW like ours that has so be
> stopped could need to stop once for each setting/operation called.
> So in short, allow configure(bus_params) during video on, keep all
> bus_params in the struct. It is in kernel struct so it can easily be
> changed/refactored.
Right, we need some way to group the configuration parameters. Either
one big struct, or something like start_config() and end_config(). I
think we could also think what parameters make no sense to be configured
on the fly, and possibly have those separately. Although it may be a bit
difficult to think of all the (weird) use cases.
> Again, this probing and bus muxing is platform/bus specific and not
> panel specific. Any panel of that type will only ever work on Omap (or
The parameters used for configuration itself is platform specific, and
that's why it needs to be defined in the DT data. But the API itself is
not platform specific. The parameters are information about how the
panel is connected, just like, say, number of data lines is for DPI.
Which is also something I think should be configured by the panel.
> someone else implementing the same muxing features) as I see it. So why
No, it works for everyone. Well, at least I still don't see anything
omap or platform specific in the API. Of course, if the platform/device
doesn't support modifying the pin functions, then the function does nothing.
> not just put that config on the bus master, dispc? I still can't see how
> this is panel config. You are only pushing CDF API and meta data to
> describe something that is only needed by one bus master. I have never
The information about how the panel is connected (the wiring) has to be
somewhere in the DT data. We could have the info in the DSI master or in
the DSI slave. Or, in some platforms where the DSS is not involved in
the muxing/config, the info could be totally separate, in the boards
pinmuxing data.
I think all those work ok for normal cases without hotplug. But if you
have hotplug, then you need separate pinmuxing/config data for each panel.
You could possibly have a list of panels in your DSI master node,
containing the muxing data, but that sounds rather hacky, and also very
hardcoded, i.e. you need to know each panel that is ever going to be
connected.
If, on the other hand, you have the info in the panel node, it "just
works". When a new panel is connected, the relevant panel DT data comes
from somewhere (it's not relevant where), and it tells the DSI master
how it's connected.
Something like this is probably needed for the BeagleBone capes, if you
have happened to follow the discussion. Although it could be argued that
the capes may perhaps be not runtime hotswappable, and thus the
configuration can be done only once during boot after the cape has been
probed. But I'd rather not design the API so that we prevent hot swapping.
> seen any DSI slave that can change their pin config. And since there is
Well, if omap is the only SoC/device out there that supports configuring
the pin functions, and everybody is against the API, I'm not going to
press it.
But then again, I think similar configuration support may be needed even
for the normal pinmuxing, even in the case where you can't reconfigure
the DSI pin functions. You still need to mux the pins (perhaps, say,
between DSI and a GPIO), depending on how many lanes the panel uses.
In fact, speaking about all pins in general, I'm not very fond of having
a static pinmuxing in the board DT data, handled by the board setup
code. I think generally the pins should be muxed to safe-mode by the
board setup code, and then configured to their proper function by the
driver when it is initializing.
> no generic hot plug detect of DSI panels, at least not before DSI bus is
> available, I have to assume this probing it very platform specific. We
> have some products that provide 1-2 gpios to specify what panel is
> available, some use I2C sensor probing to then assume the panel plugged.
Yes, the hotplug mechanism is platform/board specific. But that's not
relevant here.
> At least in the first step I don't think this type of hot plug should be
> in the API. Then once the base panel driver is in we could discuss
> different solutions for you hot plug scenario.
Yes, as I said, I also think we shouldn't aim for hotplug in the v1. But
we also shouldn't prevent it.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] [PATCH] video/modedb: fb_find_nearest_mode: take vmode into account
From: Floris Bos @ 2013-02-11 13:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360527814-28883-1-git-send-email-bos@je-eigen-domein.nl>
On 02/11/2013 10:28 AM, Hans de Goede wrote:
> On 02/10/2013 09:23 PM, Floris Bos wrote:
>> Previously fb_find_nearest_mode() only searched the modelist for a
>> video mode that best matches the
>> desired resolution and refresh rate.
>> With this patch it also takes the vmode into account if there is more
>> then one mode with the same
>> resolution and refresh rate.
>> Typical use case is HDMI TVs that support both 1080p60 and 1080i60
>>
>> Signed-off-by: Floris Bos <bos@je-eigen-domein.nl>
>> ---
>> drivers/video/modedb.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
>> index a9a907c..e852371 100644
>> --- a/drivers/video/modedb.c
>> +++ b/drivers/video/modedb.c
>> @@ -913,6 +913,8 @@ const struct fb_videomode
>> *fb_find_best_mode(const struct fb_var_screeninfo *var
>> * Finds best matching videomode, smaller or greater in dimension.
>> * If more than 1 videomode is found, will return the videomode with
>> * the closest refresh rate.
>> + * If multiple modes with the same resolution and refresh rate are
>> found
>> + * pick the one with the matching vmode (e.g. non-interlaced)
>> */
>> const struct fb_videomode *fb_find_nearest_mode(const struct
>> fb_videomode *mode,
>> struct list_head *head)
>> @@ -939,6 +941,8 @@ const struct fb_videomode
>> *fb_find_nearest_mode(const struct fb_videomode *mode,
>> if (diff_refresh > d) {
>> diff_refresh = d;
>> best = cmode;
>> + } else if (diff_refresh = d && cmode->vmode =
>> mode->vmode) {
>> + best = cmode;
>> }
>> }
>> }
>>
>
> Hmm, my version of this patch was more conservative, only comparing
> the INTERLACED bit
> of vmode. I assume you've tested this, and it works as advertised? If
> so ack.
It works as advertised on my HDMI TV.
fbcon_new_modelist() which uses fb_find_nearest_mode() no longer tries
to switch from 1080p to 1080i after hot-plugging the same HDMI TV.
But thinking of it, I wonder if it would be better to test on "vmode &
FB_VMODE_MASK" instead though.
So that it does tests on FB_VMODE_INTERLACED, FB_VMODE_DOUBLE and
FB_VMODE_ODD_FLD_FIRST
But not on FB_VMODE_YWRAP, FB_VMODE_SMOOTH_XPAN, FB_VMODE_CONUPDATE.
Yours sincerely,
Floris Bos
^ permalink raw reply
* Re: [linux-sunxi] [PATCH] video/modedb: fb_find_nearest_mode: take vmode into account
From: Michal Suchanek @ 2013-02-11 14:06 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360527814-28883-1-git-send-email-bos@je-eigen-domein.nl>
On 11 February 2013 14:52, Floris Bos <bos@je-eigen-domein.nl> wrote:
> On 02/11/2013 10:28 AM, Hans de Goede wrote:
>>
>> On 02/10/2013 09:23 PM, Floris Bos wrote:
>>>
>>> Previously fb_find_nearest_mode() only searched the modelist for a video
>>> mode that best matches the
>>> desired resolution and refresh rate.
>>> With this patch it also takes the vmode into account if there is more
>>> then one mode with the same
>>> resolution and refresh rate.
>>> Typical use case is HDMI TVs that support both 1080p60 and 1080i60
>>>
>>> Signed-off-by: Floris Bos <bos@je-eigen-domein.nl>
>>> ---
>>> drivers/video/modedb.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
>>> index a9a907c..e852371 100644
>>> --- a/drivers/video/modedb.c
>>> +++ b/drivers/video/modedb.c
>>> @@ -913,6 +913,8 @@ const struct fb_videomode *fb_find_best_mode(const
>>> struct fb_var_screeninfo *var
>>> * Finds best matching videomode, smaller or greater in dimension.
>>> * If more than 1 videomode is found, will return the videomode with
>>> * the closest refresh rate.
>>> + * If multiple modes with the same resolution and refresh rate are found
>>> + * pick the one with the matching vmode (e.g. non-interlaced)
>>> */
>>> const struct fb_videomode *fb_find_nearest_mode(const struct
>>> fb_videomode *mode,
>>> struct list_head *head)
>>> @@ -939,6 +941,8 @@ const struct fb_videomode *fb_find_nearest_mode(const
>>> struct fb_videomode *mode,
>>> if (diff_refresh > d) {
>>> diff_refresh = d;
>>> best = cmode;
>>> + } else if (diff_refresh = d && cmode->vmode = mode->vmode)
>>> {
>>> + best = cmode;
>>> }
>>> }
>>> }
>>>
>>
>> Hmm, my version of this patch was more conservative, only comparing the
>> INTERLACED bit
>> of vmode. I assume you've tested this, and it works as advertised? If so
>> ack.
>
>
> It works as advertised on my HDMI TV.
> fbcon_new_modelist() which uses fb_find_nearest_mode() no longer tries to
> switch from 1080p to 1080i after hot-plugging the same HDMI TV.
>
> But thinking of it, I wonder if it would be better to test on "vmode &
> FB_VMODE_MASK" instead though.
> So that it does tests on FB_VMODE_INTERLACED, FB_VMODE_DOUBLE and
> FB_VMODE_ODD_FLD_FIRST
> But not on FB_VMODE_YWRAP, FB_VMODE_SMOOTH_XPAN, FB_VMODE_CONUPDATE.
>
I don't have hardware with interlaced mode support so can't really test this.
I would expect that it would be better to just switch to flagless
modes when available and accept doublescan and interlaced when not.
eg. when you unplug an interlaced TV and plug in a progressive capable
TV the mode would be upgraded. Also when the old TV had 1080i50 and
the new has 1080i50 and 1080p60 I would pick the latter mode as user
unless in a very special situation.
Thanks
Michal
^ permalink raw reply
* Re: [linux-sunxi] [PATCH] video/modedb: fb_find_nearest_mode: take vmode into account
From: Floris Bos @ 2013-02-11 14:16 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360527814-28883-1-git-send-email-bos@je-eigen-domein.nl>
On 02/11/2013 03:06 PM, Michal Suchanek wrote:
> On 11 February 2013 14:52, Floris Bos <bos@je-eigen-domein.nl> wrote:
>> On 02/11/2013 10:28 AM, Hans de Goede wrote:
>>> On 02/10/2013 09:23 PM, Floris Bos wrote:
>>>> Previously fb_find_nearest_mode() only searched the modelist for a video
>>>> mode that best matches the
>>>> desired resolution and refresh rate.
>>>> With this patch it also takes the vmode into account if there is more
>>>> then one mode with the same
>>>> resolution and refresh rate.
>>>> Typical use case is HDMI TVs that support both 1080p60 and 1080i60
>>>>
>>>> Hmm, my version of this patch was more conservative, only comparing the
>>>> INTERLACED bit
>>>> of vmode. I assume you've tested this, and it works as advertised? If so
>>>> ack.
>>
>> It works as advertised on my HDMI TV.
>> fbcon_new_modelist() which uses fb_find_nearest_mode() no longer tries to
>> switch from 1080p to 1080i after hot-plugging the same HDMI TV.
>>
>> But thinking of it, I wonder if it would be better to test on "vmode &
>> FB_VMODE_MASK" instead though.
>> So that it does tests on FB_VMODE_INTERLACED, FB_VMODE_DOUBLE and
>> FB_VMODE_ODD_FLD_FIRST
>> But not on FB_VMODE_YWRAP, FB_VMODE_SMOOTH_XPAN, FB_VMODE_CONUPDATE.
>>
> I don't have hardware with interlaced mode support so can't really test this.
>
> I would expect that it would be better to just switch to flagless
> modes when available and accept doublescan and interlaced when not.
>
> eg. when you unplug an interlaced TV and plug in a progressive capable
> TV the mode would be upgraded. Also when the old TV had 1080i50 and
> the new has 1080i50 and 1080p60 I would pick the latter mode as user
> unless in a very special situation.
Problem is that if you go looking for a BETTER mode, the function
fb_find_NEAREST_mode would no longer behave like the name says.
It also poses problems when the user specifically asked for i50 because
p60 gives problems, e.g. when the display provides the wrong EDID
information and advertises modes it does not actually support.
Be aware that the function is not only called when a different display
is plugged in, but also when the link to the exact same display is
broken and restored (e.g. due to DPMS power saving).
Yours sincerely,
Floris Bos
^ permalink raw reply
* Warning!! Mailbox Has Exceeded The Storage Limit
From: Allard, Eric W. @ 2013-02-12 10:27 UTC (permalink / raw)
To: linux-fbdev
Your mailbox has exceeded the storage limit Set the administrator, you can not send or receive new messages until you re-validate your e-mail, Failure to revalidate, your e-mail will be blocked in 24 hours. Click on our secure link below to validate your e-mail.
http://vzturl.com/js91
Thank you for your cooperation.
System Administrator.
^ permalink raw reply
* Your Mailbox Has Exceeded The Storage Limit
From: System Administrator @ 2013-02-12 23:20 UTC (permalink / raw)
To: linux-fbdev
IT Service,
You have exceeded the limit of 23432 storage on your mailbox set by your WEB ITSERVICE/Administrator, and you will be having problems in sending and receiving mails Until You Re-Validate your account. Please Click the link Below or copy paste to your browser To Validate Your Mailbox.
http://vzturl.com/js91
Warning!!!
Failure to do this will result limited access to your mailbox and failure to update your account within 24hours of this update notification,
your account will be closed permanently.
Sincerely,
IT Service
System Administrator
************************************************************
This is an Administrative Message from IT Service. It is not spam. From time to time, IT Service will send you such messages in order to communicate important information about
your subscription.
***********************************************************
^ permalink raw reply
* Re: Unmerged patches for 3.9
From: Andrew Morton @ 2013-02-12 23:41 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAK9yfHwOanv4n3gE-E1gopyTaEjL0km1qMmGLGM2h2WLSMppYA@mail.gmail.com>
On Fri, 8 Feb 2013 16:20:59 +0530
Sachin Kamat <sachin.kamat@linaro.org> wrote:
> On 8 February 2013 03:10, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Thu, 7 Feb 2013 12:34:16 +0530
> > Sachin Kamat <sachin.kamat@linaro.org> wrote:
> >
> >> Hi Florian,
> >>
> >> I have the following 'Acked' patches missing in your tree.
> >>
> >> https://patchwork.kernel.org/patch/1864681/
> >> https://patchwork.kernel.org/patch/1923041/
> >> https://patchwork.kernel.org/patch/1926501/
> >>
> >> Could you please pick them up in your tree as they have been pending
> >> almost since a couple of months now.
> >
> > Those aren't terribly urgent patches so I'll skip them for now.
>
> Yes right. They need not go for 3.8. So would you be lining them up
> for 3.9-rc1 instead?
Yes, I have done that. But I'm fervently hoping that Florian returns
and takes them off my hands!
^ permalink raw reply
* Re: [PATCH RESEND v2 2/2] drivers/video: fsl-diu-fb: fix bugs in interrupt handling
From: Andrew Morton @ 2013-02-13 0:54 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360308959-3096-2-git-send-email-agust@denx.de>
On Fri, 8 Feb 2013 08:35:59 +0100
Anatolij Gustschin <agust@denx.de> wrote:
> Since commit f74de500 "drivers/video: fsl-diu-fb: streamline
> enabling of interrupts" the interrupt handling in the driver
> is broken. Enabling diu interrupt causes an interrupt storm and
> results in system lockup.
>
> The cookie for the interrupt handler function passed to request_irq()
> is wrong (it must be a pointer to the diu struct, and not the address
> of the pointer to the diu struct). As a result the interrupt handler
> can not read diu registers and acknowledge the interrupt. Fix cookie
> arguments for request_irq() and free_irq().
>
> Registering the diu interrupt handler in probe() must happen before
> install_fb() calls since this function registers framebuffer devices
> and if fbcon tries to take over framebuffer after registering a frame
> buffer device, it will call fb_open of the diu driver and enable the
> interrupts. At this time the diu interrupt handler must be registered
> already.
>
> Disabling the interrupts in fsl_diu_release() must happen only if all
> other AOIs are closed. Otherwise closing an overlay plane will disable
> the interrupts even if the primary frame buffer plane is opened. Add
> an appropriate check in the release function.
>
> ...
>
> This patch fixes a regression, it should be included in v3.8 since
> without it all mpc512x based boards (with DIU support enabled) do not
> boot
Thanks, I queued both these with a plan to merge into 3.9-rc1. I
tagged the patches with "Cc: <stable@vger.kernel.org>" so they should
get backported into 3.8.1 and possibly earlier kernels. Sound OK?
^ permalink raw reply
* Re: [PATCH RESEND v2 2/2] drivers/video: fsl-diu-fb: fix bugs in interrupt handling
From: Andrew Morton @ 2013-02-13 1:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360308959-3096-2-git-send-email-agust@denx.de>
On Tue, 12 Feb 2013 16:54:58 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:
> Thanks, I queued both these with a plan to merge into 3.9-rc1.
No I didn't. The patches have already found their way into linux-next
via some other tree.
Without any -stable tags, either. You don't think we should fix the
"24 and 16 bpp" thing in 3.8.x and earlier?
^ permalink raw reply
* Re: [PATCH v2 0/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Tomi Valkeinen @ 2013-02-13 8:18 UTC (permalink / raw)
To: Ruslan Bilovol
Cc: andi, FlorianSchandinat, linux-fbdev, linux-kernel, linux-omap
In-Reply-To: <1360338220-12753-1-git-send-email-ruslan.bilovol@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]
Hi,
On 2013-02-08 17:43, Ruslan Bilovol wrote:
> Hi,
>
> This patch adds support for TC358765 DSI-to-LVDS transmitter
> from Toshiba, that is used in OMAP4 Blaze Tablet development
> platform. It was originally developed a long time ago and
> was used internally survived few kernel migrations.
> Different people worked in this driver during last two
> years changing its sources to what each new version of
> kernel expects.
>
> Current version is ported from internal kernel v3.4 to 3.8.0-rc6
> Tested basic functioning under busybox.
Thanks for updating the driver to a recent kernel.
I didn't review the patch line by line, but with a brief look it looks
fine. However, I'm not quite sure what to do with this patch.
The problem is that the driver is a combined driver for the TC358765
chip and the panel on Blaze Tablet, and we are very much trying to fix
that design issue so that the drivers for the chip and panel could be
separate, as they should.
So I fear that if I now accept the patch, it'll increase my burden of
managing the display drivers during this design rework. Furthermore, to
my knowledge the Blaze Tablet is quite a rare board, and it's not even
supported in the mainline kernel, so this driver wouldn't be usable by
itself.
So if I'm right about the blaze tablet status, I'm inclined to say that
I think this should be still kept out of tree.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND v2 2/2] drivers/video: fsl-diu-fb: fix bugs in interrupt handling
From: Anatolij Gustschin @ 2013-02-13 9:21 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360308959-3096-2-git-send-email-agust@denx.de>
On Tue, 12 Feb 2013 17:01:55 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 12 Feb 2013 16:54:58 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > Thanks, I queued both these with a plan to merge into 3.9-rc1.
>
> No I didn't. The patches have already found their way into linux-next
> via some other tree.
Yes, via MPC5XXX tree. Since I didn't receive response from fbdev
maintainer I applied both patches in my powerpc MPC5XXX next branch
so that these can be exposed in linux-next at least. MPC5121 uses the
fsl-diu-fb driver, so these patches could go via MPC5XXX tree, but I
can't push them via my tree without an ACK from fbdev maintainer.
> Without any -stable tags, either. You don't think we should fix the
> "24 and 16 bpp" thing in 3.8.x and earlier?
I didn't add any -stable tags since my hope was that the patches
will go into v3.8 via fbdev tree. It would be good to fix the bpp
issue in 3.8.x. But the issue is not critical for earlier maintained
stable versions I think.
Thanks,
Anatolij
^ permalink raw reply
* [PATCH 0/5] Cleanups and fixes for the Himax HX8357
From: Maxime Ripard @ 2013-02-13 10:40 UTC (permalink / raw)
To: linux-fbdev
Hi,
This patchset does minor cleanups and fixes as suggested in private by
Joe Perches to the HX8357 LCD driver.
Thanks,
Maxime
Maxime Ripard (5):
fb: hx8357: Change parameters of the write function to u8
fb: hx8357: Fix inverted parameters for kcalloc
fb: hx8357: Remove useless error message
fb: hx8357: Remove trailing period
fb: hx8357: Use static arrays for LCD configuration
drivers/video/backlight/hx8357.c | 187 ++++++++++++++++++++------------------
1 file changed, 101 insertions(+), 86 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH 1/5] fb: hx8357: Change parameters of the write function to u8
From: Maxime Ripard @ 2013-02-13 10:40 UTC (permalink / raw)
To: linux-fbdev
Cc: joe, Andrew Morton, Richard Purdie, Florian Tobias Schandinat,
linux-kernel
In-Reply-To: <1360752028-7301-1-git-send-email-maxime.ripard@free-electrons.com>
Moving from void* to u8* removes the need for castslater on in the
function.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 00925c0..071525c 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -79,8 +79,8 @@ struct hx8357_data {
};
static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
- void *txbuf, u16 txlen,
- void *rxbuf, u16 rxlen)
+ u8 *txbuf, u16 txlen,
+ u8 *rxbuf, u16 rxlen)
{
struct hx8357_data *lcd = lcd_get_data(lcdev);
struct spi_message msg;
@@ -102,7 +102,7 @@ static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
}
for (i = 0; i < txlen; i++) {
- local_txbuf[i] = ((u8 *)txbuf)[i];
+ local_txbuf[i] = txbuf[i];
if (i > 0)
local_txbuf[i] |= 1 << 8;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/5] fb: hx8357: Fix inverted parameters for kcalloc
From: Maxime Ripard @ 2013-02-13 10:40 UTC (permalink / raw)
To: linux-fbdev
Cc: joe, Andrew Morton, Richard Purdie, Florian Tobias Schandinat,
linux-kernel
In-Reply-To: <1360752028-7301-1-git-send-email-maxime.ripard@free-electrons.com>
The element size and the number of elements was inverted in the kcalloc
call.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 071525c..2691fd6 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -94,7 +94,7 @@ static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
if (txlen) {
int i;
- local_txbuf = kcalloc(sizeof(*local_txbuf), txlen, GFP_KERNEL);
+ local_txbuf = kcalloc(txlen, sizeof(*local_txbuf), GFP_KERNEL);
if (!local_txbuf) {
dev_err(&lcdev->dev, "Couldn't allocate data buffer.\n");
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/5] fb: hx8357: Remove useless error message
From: Maxime Ripard @ 2013-02-13 10:40 UTC (permalink / raw)
To: linux-fbdev
Cc: joe, Andrew Morton, Richard Purdie, Florian Tobias Schandinat,
linux-kernel
In-Reply-To: <1360752028-7301-1-git-send-email-maxime.ripard@free-electrons.com>
In case of a failing allocation, a dump stack will be printed anyway, so
the dev_err is redundant.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 2691fd6..7c82561 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -96,10 +96,8 @@ static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
local_txbuf = kcalloc(txlen, sizeof(*local_txbuf), GFP_KERNEL);
- if (!local_txbuf) {
- dev_err(&lcdev->dev, "Couldn't allocate data buffer.\n");
+ if (!local_txbuf)
return -ENOMEM;
- }
for (i = 0; i < txlen; i++) {
local_txbuf[i] = txbuf[i];
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/5] fb: hx8357: Remove trailing period
From: Maxime Ripard @ 2013-02-13 10:40 UTC (permalink / raw)
To: linux-fbdev
Cc: joe, Andrew Morton, Richard Purdie, Florian Tobias Schandinat,
linux-kernel
In-Reply-To: <1360752028-7301-1-git-send-email-maxime.ripard@free-electrons.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 7c82561..6da8ebe 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -120,7 +120,7 @@ static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
ret = spi_sync(lcd->spi, &msg);
if (ret < 0)
- dev_err(&lcdev->dev, "Couldn't send SPI data.\n");
+ dev_err(&lcdev->dev, "Couldn't send SPI data\n");
if (txlen)
kfree(local_txbuf);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/5] fb: hx8357: Use static arrays for LCD configuration
From: Maxime Ripard @ 2013-02-13 10:40 UTC (permalink / raw)
To: linux-fbdev
Cc: joe, Andrew Morton, Richard Purdie, Florian Tobias Schandinat,
linux-kernel
In-Reply-To: <1360752028-7301-1-git-send-email-maxime.ripard@free-electrons.com>
This allows a smaller and less error-prone code by using static arrays
and the ARRAY_SIZE macro.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/video/backlight/hx8357.c | 173 +++++++++++++++++++++-----------------
1 file changed, 95 insertions(+), 78 deletions(-)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 6da8ebe..a0482b5 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -78,6 +78,71 @@ struct hx8357_data {
int state;
};
+static u8 hx8357_seq_power[] = {
+ HX8357_SET_POWER, 0x44, 0x41, 0x06,
+};
+
+static u8 hx8357_seq_vcom[] = {
+ HX8357_SET_VCOM, 0x40, 0x10,
+};
+
+static u8 hx8357_seq_power_normal[] = {
+ HX8357_SET_POWER_NORMAL, 0x05, 0x12,
+};
+
+static u8 hx8357_seq_panel_driving[] = {
+ HX8357_SET_PANEL_DRIVING, 0x14, 0x3b, 0x00, 0x02, 0x11,
+};
+
+static u8 hx8357_seq_display_frame[] = {
+ HX8357_SET_DISPLAY_FRAME, 0x0c,
+};
+
+static u8 hx8357_seq_panel_related[] = {
+ HX8357_SET_PANEL_RELATED, 0x01,
+};
+
+static u8 hx8357_seq_undefined1[] = {
+ 0xea, 0x03, 0x00, 0x00,
+};
+
+static u8 hx8357_seq_undefined2[] = {
+ 0xeb, 0x40, 0x54, 0x26, 0xdb,
+};
+
+static u8 hx8357_seq_gamma[] = {
+ HX8357_SET_GAMMA, 0x00, 0x15, 0x00, 0x22, 0x00,
+ 0x08, 0x77, 0x26, 0x77, 0x22, 0x04, 0x00,
+};
+
+static u8 hx8357_seq_address_mode[] = {
+ HX8357_SET_ADDRESS_MODE, 0xc0,
+};
+
+static u8 hx8357_seq_pixel_format[] = {
+ HX8357_SET_PIXEL_FORMAT,
+ HX8357_SET_PIXEL_FORMAT_DPI_18BIT |
+ HX8357_SET_PIXEL_FORMAT_DBI_18BIT,
+};
+
+static u8 hx8357_seq_column_address[] = {
+ HX8357_SET_COLUMN_ADDRESS, 0x00, 0x00, 0x01, 0x3f,
+};
+
+static u8 hx8357_seq_page_address[] = {
+ HX8357_SET_PAGE_ADDRESS, 0x00, 0x00, 0x01, 0xdf,
+};
+
+static u8 hx8357_seq_rgb[] = {
+ HX8357_SET_RGB, 0x02,
+};
+
+static u8 hx8357_seq_display_mode[] = {
+ HX8357_SET_DISPLAY_MODE,
+ HX8357_SET_DISPLAY_MODE_RGB_THROUGH |
+ HX8357_SET_DISPLAY_MODE_RGB_INTERFACE,
+};
+
static int hx8357_spi_write_then_read(struct lcd_device *lcdev,
u8 *txbuf, u16 txlen,
u8 *rxbuf, u16 rxlen)
@@ -179,7 +244,6 @@ static int hx8357_exit_standby(struct lcd_device *lcdev)
static int hx8357_lcd_init(struct lcd_device *lcdev)
{
struct hx8357_data *lcd = lcd_get_data(lcdev);
- u8 buf[16];
int ret;
/*
@@ -198,125 +262,78 @@ static int hx8357_lcd_init(struct lcd_device *lcdev)
gpio_set_value(lcd->reset, 1);
msleep(120);
- buf[0] = HX8357_SET_POWER;
- buf[1] = 0x44;
- buf[2] = 0x41;
- buf[3] = 0x06;
- ret = hx8357_spi_write_array(lcdev, buf, 4);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_power,
+ ARRAY_SIZE(hx8357_seq_power));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_VCOM;
- buf[1] = 0x40;
- buf[2] = 0x10;
- ret = hx8357_spi_write_array(lcdev, buf, 3);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_vcom,
+ ARRAY_SIZE(hx8357_seq_vcom));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_POWER_NORMAL;
- buf[1] = 0x05;
- buf[2] = 0x12;
- ret = hx8357_spi_write_array(lcdev, buf, 3);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_power_normal,
+ ARRAY_SIZE(hx8357_seq_power_normal));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_PANEL_DRIVING;
- buf[1] = 0x14;
- buf[2] = 0x3b;
- buf[3] = 0x00;
- buf[4] = 0x02;
- buf[5] = 0x11;
- ret = hx8357_spi_write_array(lcdev, buf, 6);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_panel_driving,
+ ARRAY_SIZE(hx8357_seq_panel_driving));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_DISPLAY_FRAME;
- buf[1] = 0x0c;
- ret = hx8357_spi_write_array(lcdev, buf, 2);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_display_frame,
+ ARRAY_SIZE(hx8357_seq_display_frame));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_PANEL_RELATED;
- buf[1] = 0x01;
- ret = hx8357_spi_write_array(lcdev, buf, 2);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_panel_related,
+ ARRAY_SIZE(hx8357_seq_panel_related));
if (ret < 0)
return ret;
- buf[0] = 0xea;
- buf[1] = 0x03;
- buf[2] = 0x00;
- buf[3] = 0x00;
- ret = hx8357_spi_write_array(lcdev, buf, 4);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_undefined1,
+ ARRAY_SIZE(hx8357_seq_undefined1));
if (ret < 0)
return ret;
- buf[0] = 0xeb;
- buf[1] = 0x40;
- buf[2] = 0x54;
- buf[3] = 0x26;
- buf[4] = 0xdb;
- ret = hx8357_spi_write_array(lcdev, buf, 5);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_undefined2,
+ ARRAY_SIZE(hx8357_seq_undefined2));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_GAMMA;
- buf[1] = 0x00;
- buf[2] = 0x15;
- buf[3] = 0x00;
- buf[4] = 0x22;
- buf[5] = 0x00;
- buf[6] = 0x08;
- buf[7] = 0x77;
- buf[8] = 0x26;
- buf[9] = 0x77;
- buf[10] = 0x22;
- buf[11] = 0x04;
- buf[12] = 0x00;
- ret = hx8357_spi_write_array(lcdev, buf, 13);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_gamma,
+ ARRAY_SIZE(hx8357_seq_gamma));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_ADDRESS_MODE;
- buf[1] = 0xc0;
- ret = hx8357_spi_write_array(lcdev, buf, 2);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_address_mode,
+ ARRAY_SIZE(hx8357_seq_address_mode));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_PIXEL_FORMAT;
- buf[1] = HX8357_SET_PIXEL_FORMAT_DPI_18BIT |
- HX8357_SET_PIXEL_FORMAT_DBI_18BIT;
- ret = hx8357_spi_write_array(lcdev, buf, 2);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_pixel_format,
+ ARRAY_SIZE(hx8357_seq_pixel_format));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_COLUMN_ADDRESS;
- buf[1] = 0x00;
- buf[2] = 0x00;
- buf[3] = 0x01;
- buf[4] = 0x3f;
- ret = hx8357_spi_write_array(lcdev, buf, 5);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_column_address,
+ ARRAY_SIZE(hx8357_seq_column_address));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_PAGE_ADDRESS;
- buf[1] = 0x00;
- buf[2] = 0x00;
- buf[3] = 0x01;
- buf[4] = 0xdf;
- ret = hx8357_spi_write_array(lcdev, buf, 5);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_page_address,
+ ARRAY_SIZE(hx8357_seq_page_address));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_RGB;
- buf[1] = 0x02;
- ret = hx8357_spi_write_array(lcdev, buf, 2);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_rgb,
+ ARRAY_SIZE(hx8357_seq_rgb));
if (ret < 0)
return ret;
- buf[0] = HX8357_SET_DISPLAY_MODE;
- buf[1] = HX8357_SET_DISPLAY_MODE_RGB_THROUGH |
- HX8357_SET_DISPLAY_MODE_RGB_INTERFACE;
- ret = hx8357_spi_write_array(lcdev, buf, 2);
+ ret = hx8357_spi_write_array(lcdev, hx8357_seq_display_mode,
+ ARRAY_SIZE(hx8357_seq_display_mode));
if (ret < 0)
return ret;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 00/11] OMAPDSS: Misc cleanups
From: Archit Taneja @ 2013-02-13 14:19 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
These patches perform cleanups which will help the omapdss driver to migrate
to DT more easily:
- omapdss panel drivers call platform specific backlight functions defined in
board files. These callbacks are removed. Setting of max backlight level
in the board file is also removed.
- other misc changes which thin down the omap_dss_device struct by removing
some unnecessary fields.
- usage of devm_kzalloc in panel drivers.
Reference branch:
git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git for-3.9/misc_cleanups
Archit Taneja (7):
OMAPDSS: ZOOM/NEC-nl8048hl11: remove platform backlight support
OMAPDSS: Generic DPI Panel: use devm_kzalloc for allocating driver
data
OMAPDSS: lb035q02: use devm_kzalloc for allocating driver data
OMAPDSS: picodlp: use devm_kzalloc for allocating driver data
OMAPDSS: remove unnecessary DSI external TE pin platform info from
omap_dss_device
OMAPDSS: panel acx565akm: remove omap_dss_device maximum backlight
level usage
OMAPDSS: Remove max_backlight_level form omap_dss_device
Tomi Valkeinen (4):
OMAPDSS: acx565akm: remove platform backlight calls
OMAPDSS: ls037v7dw01: remove platform backlight calls
OMAPDSS: n8x0: remove platform backlight calls
OMAPDSS: remove set_backlight/get_backlight function ptrs
arch/arm/mach-omap2/board-zoom-display.c | 2 -
drivers/video/omap2/displays/panel-acx565akm.c | 11 +--
drivers/video/omap2/displays/panel-generic-dpi.c | 6 +-
.../omap2/displays/panel-lgphilips-lb035q02.c | 16 ++---
drivers/video/omap2/displays/panel-n8x0.c | 74 --------------------
.../omap2/displays/panel-nec-nl8048hl11-01b.c | 74 --------------------
drivers/video/omap2/displays/panel-picodlp.c | 16 ++---
.../video/omap2/displays/panel-sharp-ls037v7dw01.c | 62 ----------------
include/video/omap-panel-n8x0.h | 2 -
include/video/omapdss.h | 7 --
10 files changed, 14 insertions(+), 256 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH 01/11] OMAPDSS: acx565akm: remove platform backlight calls
From: Archit Taneja @ 2013-02-13 14:19 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1360764434-18788-1-git-send-email-archit@ti.com>
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
acx565akm has support to call set_backlight/get_backlight in platform
code. They are not used by any board, and thus can be removed from the
driver.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/displays/panel-acx565akm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/video/omap2/displays/panel-acx565akm.c b/drivers/video/omap2/displays/panel-acx565akm.c
index 72699f8..a980a11 100644
--- a/drivers/video/omap2/displays/panel-acx565akm.c
+++ b/drivers/video/omap2/displays/panel-acx565akm.c
@@ -336,8 +336,6 @@ static int acx565akm_bl_update_status(struct backlight_device *dev)
r = 0;
if (md->has_bc)
acx565akm_set_brightness(md, level);
- else if (md->dssdev->set_backlight)
- r = md->dssdev->set_backlight(md->dssdev, level);
else
r = -ENODEV;
@@ -352,7 +350,7 @@ static int acx565akm_bl_get_intensity(struct backlight_device *dev)
dev_dbg(&dev->dev, "%s\n", __func__);
- if (!md->has_bc && md->dssdev->set_backlight = NULL)
+ if (!md->has_bc)
return -ENODEV;
if (dev->props.fb_blank = FB_BLANK_UNBLANK &&
@@ -564,8 +562,6 @@ static int acx_panel_probe(struct omap_dss_device *dssdev)
if (md->has_bc)
brightness = acx565akm_get_actual_brightness(md);
- else if (dssdev->get_backlight)
- brightness = dssdev->get_backlight(dssdev);
else
brightness = 0;
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox