* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21 11:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <13540495.epaCf4JVn9@percival>
On 2012-11-21 06:23, Alex Courbot wrote:
> Hi Grant,
>
> On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
>>> With the advent of the device tree and of ARM kernels that are not
>>> board-tied, we cannot rely on these board-specific hooks anymore but
>>
>> This isn't strictly true. It is still perfectly fine to have board
>> specific code when necessary. However, there is strong encouragement to
>> enable that code in device drivers as much as possible and new board
>> files need to have very strong justification.
>
> But doesn't introducing board-specific code into the kernel just defeats the
> purpose of the DT? If we extend this logic, we are heading straight back to
> board-definition files. To a lesser extent than before I agree, but the problem
> is fundamentally the same.
I don't think so. I'll reiterate my opinion on this subject, as a
summary and for those who haven't read the discussions of the earlier
versions of the series. And perhaps I'll even manage to say something
new =).
First about the board specific code. I think we may need some board
specific code, even if this series was merged. Let's call them board
drivers. These board drivers would only exist for boards with some weird
setups that cannot be expressed or managed with DT and normal drivers.
I think these cases would be quite rare, as I can't even come up with a
very good example. I guess most likely these cases would involve some
small trivial chips for muxing or such, for which it doesn't really make
sense to have a real driver.
Say, perhaps a board with two LCDs connected to one video output, and
only one LCD can be enabled at a time, and you need to set some mux chip
to route the signals to the correct LCD. In this case I'd see we should
have hotplug support in the display framework, and the board driver
would act on user input (sysfs file, perhaps), plugging in/out the LCD
device depending on the user input.
As for expressing device internal details in the DT data, as done in
this series, I don't like it very much. I think the DT data or the board
file should just describe which device is the one in question, and how
it's connected to other components on the board. The driver for the
device should handle everything else.
As Alex pointed out, there may be lots of devices that work the same way
when enabled, but require slightly different power-on/off sequences. We
could have these sequences in the driver itself, either as plain code,
or in a table of some sort, possibly using the power sequence framework
presented in this series. The correct code or sequence would be ran
depending on the particular model of the device.
I think this approach is correct in the sense that this is what drivers
are supposed to do: handle all the device internal matters. But this
raises the problem of bloating the kernel with possibly lots of
different power sequences, of which only a few would be used by a board,
and all the rest would just be waste of memory.
Regarding this problem I have the following questions, to which I don't
have clear answers:
- How much of this device specific data is too much? If a driver
supports 10 different models, and the sequences for each model take 100
bytes, this 1000 bytes doesn't sound too much. But where's the limit?
And even if one driver only has 1kB of this data, what if we have lots
of similar drivers?
- How many bytes does a power sequence presented in this series take, if
the sequence contains, say, 5 steps, with gpio, delay, pwm, delay and
regulator?
- How likely is it that we'll get lots of different models? A hundred
different models for a backlight PWM with different power-on/off
sequences already sounds a really big number. If we're only going to
have each driver supporting, say, no more than 10 models, perhaps the
problem is not even an issue in practice.
- Are there ways to limit this bloat in the driver? One obvious way
would be to discard the unused sequences after driver probe, but that
only works for platform devices. Although, I guess these sequences would
mostly be used by platform drivers? If so, then the problem could be
solved by discarding the data after probe. Another would be to load the
sequences from a file as firmware, but that feels quite an awful
solution. Anybody have other ideas?
One clear positive side with in-driver approach is that it's totally
inside the kernel, and can be easily reworked in the future, or even
changed to a DT-based approach as presented in this series if that seems
like a better solution. The DT based approach, on the other hand, will
be more or less written to stone after it's merged.
So, I like the framework of expressing the sequences presented in this
series (except there's a problem with error cases, as I pointed out in
another post), as it can be used inside the drivers. But I'm not so
enthusiastic about presenting the sequences in DT.
My suggestion would be to go forward with an in-driver solution, and
look at the DT based solution later if we are seeing an increasing bloat
in the drivers.
Tomi
^ permalink raw reply
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Leela Krishna Amudala @ 2012-11-21 11:21 UTC (permalink / raw)
To: Manjunathappa, Prakash
Cc: Steffen Trumtrar, devicetree-discuss@lists.ozlabs.org,
linux-fbdev@vger.kernel.org, David Airlie,
Florian Tobias Schandinat, dri-devel@lists.freedesktop.org,
Valkeinen, Tomi, Laurent Pinchart, kernel@pengutronix.de,
Guennady Liakhovetski, linux-media@vger.kernel.org
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9FA769@DBDE01.ent.ti.com>
Yes,
Even I got the same build error.
later I fixed it by including "#include <linux/mxsfb.h>"
Best Wishes,
Leela Krishna.
On Wed, Nov 21, 2012 at 3:39 PM, Manjunathappa, Prakash
<prakash.pm@ti.com> wrote:
> Hi Steffen,
>
> I am trying to add DT support for da8xx-fb driver on top of your patches.
> Encountered below build error. Sorry for reporting it late.
>
> On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote:
>> Add a function to convert from the generic videomode to a fb_videomode.
>>
>> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
>> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
>> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
>> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>> drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/fb.h | 6 ++++++
>> 2 files changed, 52 insertions(+)
>>
>> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
>> index cef6557..c1939a6 100644
>> --- a/drivers/video/fbmon.c
>> +++ b/drivers/video/fbmon.c
>> @@ -31,6 +31,7 @@
>> #include <linux/pci.h>
>> #include <linux/slab.h>
>> #include <video/edid.h>
>> +#include <linux/videomode.h>
>> #ifdef CONFIG_PPC_OF
>> #include <asm/prom.h>
>> #include <asm/pci-bridge.h>
>> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>> kfree(timings);
>> return err;
>> }
>> +
>> +#if IS_ENABLED(CONFIG_VIDEOMODE)
>> +int fb_videomode_from_videomode(const struct videomode *vm,
>> + struct fb_videomode *fbmode)
>> +{
>> + unsigned int htotal, vtotal;
>> +
>> + fbmode->xres = vm->hactive;
>> + fbmode->left_margin = vm->hback_porch;
>> + fbmode->right_margin = vm->hfront_porch;
>> + fbmode->hsync_len = vm->hsync_len;
>> +
>> + fbmode->yres = vm->vactive;
>> + fbmode->upper_margin = vm->vback_porch;
>> + fbmode->lower_margin = vm->vfront_porch;
>> + fbmode->vsync_len = vm->vsync_len;
>> +
>> + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
>> +
>> + fbmode->sync = 0;
>> + fbmode->vmode = 0;
>> + if (vm->hah)
>> + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
>> + if (vm->vah)
>> + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
>> + if (vm->interlaced)
>> + fbmode->vmode |= FB_VMODE_INTERLACED;
>> + if (vm->doublescan)
>> + fbmode->vmode |= FB_VMODE_DOUBLE;
>> + if (vm->de)
>> + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
>
> "FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting
> build error on this. Please let me know if I am missing something.
>
> Thanks,
> Prakash
>
>> + fbmode->flag = 0;
>> +
>> + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
>> + vm->hsync_len;
>> + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
>> + vm->vsync_len;
>> + fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
>> +#endif
>> +
>> +
>> #else
>> int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>> {
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index c7a9571..920cbe3 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -14,6 +14,7 @@
>> #include <linux/backlight.h>
>> #include <linux/slab.h>
>> #include <asm/io.h>
>> +#include <linux/videomode.h>
>>
>> struct vm_area_struct;
>> struct fb_info;
>> @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>> extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>> extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>>
>> +#if IS_ENABLED(CONFIG_VIDEOMODE)
>> +extern int fb_videomode_from_videomode(const struct videomode *vm,
>> + struct fb_videomode *fbmode);
>> +#endif
>> +
>> /* drivers/video/modedb.c */
>> #define VESA_MODEDB_SIZE 34
>> extern void fb_var_to_videomode(struct fb_videomode *mode,
>> --
>> 1.7.10.4
>>
>> --
>> 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
>>
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply
* Re: [PATCH v12 1/6] video: add display_timing and videomode
From: Tomi Valkeinen @ 2012-11-21 11:37 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353426896-6045-2-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6621 bytes --]
Hi,
On 2012-11-20 17:54, Steffen Trumtrar wrote:
> Add display_timing structure and the according helper functions. This allows
> the description of a display via its supported timing parameters.
>
> Every timing parameter can be specified as a single value or a range
> <min typ max>.
>
> Also, add helper functions to convert from display timings to a generic videomode
> structure. This videomode can then be converted to the corresponding subsystem
> mode representation (e.g. fb_videomode).
Sorry for reviewing this so late.
One thing I'd like to see is some explanation of the structs involved.
For example, in this patch you present structs videomode, display_timing
and display_timings without giving any hint what they represent.
I'm not asking for you to write a long documentation, but perhaps the
header files could include a few lines of comments above the structs,
explaining the idea.
> +void display_timings_release(struct display_timings *disp)
> +{
> + if (disp->timings) {
> + unsigned int i;
> +
> + for (i = 0; i < disp->num_timings; i++)
> + kfree(disp->timings[i]);
> + kfree(disp->timings);
> + }
> + kfree(disp);
> +}
> +EXPORT_SYMBOL_GPL(display_timings_release);
Perhaps this will become clearer after reading the following patches,
but it feels a bit odd to add a release function, without anything in
this patch that would actually allocate the timings.
> diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
> new file mode 100644
> index 0000000..e24f879
> --- /dev/null
> +++ b/drivers/video/videomode.c
> @@ -0,0 +1,46 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/export.h>
> +#include <linux/errno.h>
> +#include <linux/display_timing.h>
> +#include <linux/kernel.h>
> +#include <linux/videomode.h>
> +
> +int videomode_from_timing(const struct display_timings *disp,
> + struct videomode *vm, unsigned int index)
> +{
> + struct display_timing *dt;
> +
> + dt = display_timings_get(disp, index);
> + if (!dt)
> + return -EINVAL;
> +
> + vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
> + vm->hactive = display_timing_get_value(&dt->hactive, 0);
> + vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
> + vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
> + vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
> +
> + vm->vactive = display_timing_get_value(&dt->vactive, 0);
> + vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
> + vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
> + vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);
Shouldn't all these calls get the typical value, with index 1?
> +
> + vm->vah = dt->vsync_pol_active;
> + vm->hah = dt->hsync_pol_active;
> + vm->de = dt->de_pol_active;
> + vm->pixelclk_pol = dt->pixelclk_pol;
> +
> + vm->interlaced = dt->interlaced;
> + vm->doublescan = dt->doublescan;
> +
> + return 0;
> +}
> +
> +EXPORT_SYMBOL_GPL(videomode_from_timing);
> diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
> new file mode 100644
> index 0000000..d5bf03f
> --- /dev/null
> +++ b/include/linux/display_timing.h
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * description of display timings
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_DISPLAY_TIMINGS_H
> +#define __LINUX_DISPLAY_TIMINGS_H
> +
> +#include <linux/types.h>
What is this needed for? u32? I don't see it defined in types.h
> +
> +struct timing_entry {
> + u32 min;
> + u32 typ;
> + u32 max;
> +};
> +
> +struct display_timing {
> + struct timing_entry pixelclock;
> +
> + struct timing_entry hactive;
> + struct timing_entry hfront_porch;
> + struct timing_entry hback_porch;
> + struct timing_entry hsync_len;
> +
> + struct timing_entry vactive;
> + struct timing_entry vfront_porch;
> + struct timing_entry vback_porch;
> + struct timing_entry vsync_len;
> +
> + unsigned int vsync_pol_active;
> + unsigned int hsync_pol_active;
> + unsigned int de_pol_active;
> + unsigned int pixelclk_pol;
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +struct display_timings {
> + unsigned int num_timings;
> + unsigned int native_mode;
> +
> + struct display_timing **timings;
> +};
> +
> +/*
> + * placeholder function until ranges are really needed
> + * the index parameter should then be used to select one of [min typ max]
> + */
> +static inline u32 display_timing_get_value(const struct timing_entry *te,
> + unsigned int index)
> +{
> + return te->typ;
> +}
Why did you opt for a placeholder here? It feels trivial to me to have
support to get the min/typ/max value properly.
> +static inline struct display_timing *display_timings_get(const struct
> + display_timings *disp,
> + unsigned int index)
> +{
> + if (disp->num_timings > index)
> + return disp->timings[index];
> + else
> + return NULL;
> +}
> +
> +void display_timings_release(struct display_timings *disp);
> +
> +#endif
> diff --git a/include/linux/videomode.h b/include/linux/videomode.h
> new file mode 100644
> index 0000000..5d3e796
> --- /dev/null
> +++ b/include/linux/videomode.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * generic videomode description
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_VIDEOMODE_H
> +#define __LINUX_VIDEOMODE_H
> +
> +#include <linux/display_timing.h>
This is not needed, just add:
struct display_timings;
> +struct videomode {
> + u32 pixelclock;
> + u32 refreshrate;
> +
> + u32 hactive;
> + u32 hfront_porch;
> + u32 hback_porch;
> + u32 hsync_len;
> +
> + u32 vactive;
> + u32 vfront_porch;
> + u32 vback_porch;
> + u32 vsync_len;
> +
> + u32 hah;
> + u32 vah;
> + u32 de;
> + u32 pixelclk_pol;
> +
> + bool interlaced;
> + bool doublescan;
> +};
> +
> +int videomode_from_timing(const struct display_timings *disp,
> + struct videomode *vm, unsigned int index);
> +
Are this and the few other functions above meant to be used from
drivers? If so, some explanation of the parameters here would be nice.
If they are just framework internal, they don't probably need that.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-11-21 11:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50ACB59B.4090404@iki.fi>
[-- Attachment #1: Type: text/plain, Size: 8080 bytes --]
On Wed, Nov 21, 2012 at 01:06:03PM +0200, Tomi Valkeinen wrote:
> On 2012-11-21 06:23, Alex Courbot wrote:
> > Hi Grant,
> >
> > On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
> >>> With the advent of the device tree and of ARM kernels that are not
> >>> board-tied, we cannot rely on these board-specific hooks anymore but
> >>
> >> This isn't strictly true. It is still perfectly fine to have board
> >> specific code when necessary. However, there is strong encouragement to
> >> enable that code in device drivers as much as possible and new board
> >> files need to have very strong justification.
> >
> > But doesn't introducing board-specific code into the kernel just defeats the
> > purpose of the DT? If we extend this logic, we are heading straight back to
> > board-definition files. To a lesser extent than before I agree, but the problem
> > is fundamentally the same.
>
> I don't think so. I'll reiterate my opinion on this subject, as a
> summary and for those who haven't read the discussions of the earlier
> versions of the series. And perhaps I'll even manage to say something
> new =).
>
>
> First about the board specific code. I think we may need some board
> specific code, even if this series was merged. Let's call them board
> drivers. These board drivers would only exist for boards with some weird
> setups that cannot be expressed or managed with DT and normal drivers.
>
> I think these cases would be quite rare, as I can't even come up with a
> very good example. I guess most likely these cases would involve some
> small trivial chips for muxing or such, for which it doesn't really make
> sense to have a real driver.
>
> Say, perhaps a board with two LCDs connected to one video output, and
> only one LCD can be enabled at a time, and you need to set some mux chip
> to route the signals to the correct LCD. In this case I'd see we should
> have hotplug support in the display framework, and the board driver
> would act on user input (sysfs file, perhaps), plugging in/out the LCD
> device depending on the user input.
>
>
> As for expressing device internal details in the DT data, as done in
> this series, I don't like it very much. I think the DT data or the board
> file should just describe which device is the one in question, and how
> it's connected to other components on the board. The driver for the
> device should handle everything else.
>
> As Alex pointed out, there may be lots of devices that work the same way
> when enabled, but require slightly different power-on/off sequences. We
> could have these sequences in the driver itself, either as plain code,
> or in a table of some sort, possibly using the power sequence framework
> presented in this series. The correct code or sequence would be ran
> depending on the particular model of the device.
>
> I think this approach is correct in the sense that this is what drivers
> are supposed to do: handle all the device internal matters. But this
> raises the problem of bloating the kernel with possibly lots of
> different power sequences, of which only a few would be used by a board,
> and all the rest would just be waste of memory.
>
> Regarding this problem I have the following questions, to which I don't
> have clear answers:
>
> - How much of this device specific data is too much? If a driver
> supports 10 different models, and the sequences for each model take 100
> bytes, this 1000 bytes doesn't sound too much. But where's the limit?
> And even if one driver only has 1kB of this data, what if we have lots
> of similar drivers?
>
> - How many bytes does a power sequence presented in this series take, if
> the sequence contains, say, 5 steps, with gpio, delay, pwm, delay and
> regulator?
>
> - How likely is it that we'll get lots of different models? A hundred
> different models for a backlight PWM with different power-on/off
> sequences already sounds a really big number. If we're only going to
> have each driver supporting, say, no more than 10 models, perhaps the
> problem is not even an issue in practice.
>
> - Are there ways to limit this bloat in the driver? One obvious way
> would be to discard the unused sequences after driver probe, but that
> only works for platform devices. Although, I guess these sequences would
> mostly be used by platform drivers? If so, then the problem could be
> solved by discarding the data after probe. Another would be to load the
> sequences from a file as firmware, but that feels quite an awful
> solution. Anybody have other ideas?
>
> One clear positive side with in-driver approach is that it's totally
> inside the kernel, and can be easily reworked in the future, or even
> changed to a DT-based approach as presented in this series if that seems
> like a better solution. The DT based approach, on the other hand, will
> be more or less written to stone after it's merged.
>
>
> So, I like the framework of expressing the sequences presented in this
> series (except there's a problem with error cases, as I pointed out in
> another post), as it can be used inside the drivers. But I'm not so
> enthusiastic about presenting the sequences in DT.
>
> My suggestion would be to go forward with an in-driver solution, and
> look at the DT based solution later if we are seeing an increasing bloat
> in the drivers.
Assuming we go with your approach, what's the plan? We're actually
facing this problem right now for Tegra. Basically we have a DRM driver
that can drive the panel, but we're still missing a way to hook up the
backlight and panel enabling code. So we effectively can't support any
of the LVDS devices out there without this series.
As I understand it, what you propose is similar to what ASoC does. For a
specific board, you'd have to write a driver, presumably for the new
panel/display framework, that provides code to power the panel on and
off. That means we'll have to have a driver for each panel out there
basically, or we'd need to write generic drivers that can be configured
to some degree (via platform data or DT). This is similar to how ASoC
works, where we have a driver that provides support for a specific codec
connected to the Tegra SoC. For the display framework things could be
done in a similar way I suppose, so that Tegra could have one display
driver to handle all aspects of powering on and off the various panels
for the various boards out there.
Obviously, a lot of the code will be similar for other SoCs, but maybe
that's just the way things are if we choose that approach. There's also
the potential for factoring out large chunks of common code later on
once we start to see common patterns.
One thing that's not very clear is how the backlight subsystem should be
wired up with the display framework. I have a patch on top of the Tegra
DRM driver which adds some ad-hoc display support using this power
sequences series and the pwm-backlight.
From reading the proposal for the panel/display framework, it sounds
like a lot more is planned than just enabling or disabling panels, but
it also seems like a lot of code needs to be written to support things
like DSI, DBI or other control busses.
At least for Tegra, and I think the same holds for a wide variety of
other SoCs, dumb panels would be enough for a start. In the interest of
getting a working solution for those setups, maybe we can start small
and add just enough framework to register dumb panel drivers to along
with code to wire up a backlight to light up the display. Then we could
possibly still make it to have a proper solution to support the various
LVDS panels for Tegra with 3.9.
I'm adding Laurent on Cc since he's probably busy working on a new
proposal for the panel/display framework. Maybe he can share his thought
on this.
All of the above said, I'm willing to help out with the coding if that's
what is required to reach a solution that everybody can be happy with.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Steffen Trumtrar @ 2012-11-21 11:48 UTC (permalink / raw)
To: Manjunathappa, Prakash
Cc: devicetree-discuss@lists.ozlabs.org, Philipp Zabel, Rob Herring,
linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media@vger.kernel.org, Valkeinen, Tomi, Stephen Warren,
kernel@pengutronix.de, Florian Tobias Schandinat, David Airlie
In-Reply-To: <A73F36158E33644199EB82C5EC81C7BC3E9FA7A0@DBDE01.ent.ti.com>
Hi!
On Wed, Nov 21, 2012 at 10:12:43AM +0000, Manjunathappa, Prakash wrote:
> Hi Steffen,
>
> On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
> > +/**
> > + * of_get_display_timings - parse all display_timing entries from a device_node
> > + * @np: device_node with the subnodes
> > + **/
> > +struct display_timings *of_get_display_timings(const struct device_node *np)
> > +{
> > + struct device_node *timings_np;
> > + struct device_node *entry;
> > + struct device_node *native_mode;
> > + struct display_timings *disp;
> > +
> > + if (!np) {
> > + pr_err("%s: no devicenode given\n", __func__);
> > + return NULL;
> > + }
> > +
> > + timings_np = of_find_node_by_name(np, "display-timings");
>
> I get below build warnings on this line
> drivers/video/of_display_timing.c: In function 'of_get_display_timings':
> drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of 'of_find_node_by_name' discards qualifiers from pointer target type
> include/linux/of.h:167:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
>
> > + * of_display_timings_exists - check if a display-timings node is provided
> > + * @np: device_node with the timing
> > + **/
> > +int of_display_timings_exists(const struct device_node *np)
> > +{
> > + struct device_node *timings_np;
> > +
> > + if (!np)
> > + return -EINVAL;
> > +
> > + timings_np = of_parse_phandle(np, "display-timings", 0);
>
> Also here:
> drivers/video/of_display_timing.c: In function 'of_display_timings_exists':
> drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of 'of_parse_phandle' discards qualifiers from pointer target type
> include/linux/of.h:258:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
>
The warnings are because the of-functions do not use const pointers where they
should. I had two options: don't use const pointers even if they should be and
have no warnings or use const pointers and have a correct API. (Third option:
send patches for of-functions). I chose the second option.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Thierry Reding @ 2012-11-21 11:52 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: Manjunathappa, Prakash, devicetree-discuss@lists.ozlabs.org,
Philipp Zabel, Rob Herring, linux-fbdev@vger.kernel.org,
dri-devel@lists.freedesktop.org, Laurent Pinchart,
Guennady Liakhovetski, linux-media@vger.kernel.org,
Valkeinen, Tomi, Stephen Warren, kernel@pengutronix.de,
Florian Tobias Schandinat, David Airlie
In-Reply-To: <20121121114843.GC14013@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2568 bytes --]
On Wed, Nov 21, 2012 at 12:48:43PM +0100, Steffen Trumtrar wrote:
> Hi!
>
> On Wed, Nov 21, 2012 at 10:12:43AM +0000, Manjunathappa, Prakash wrote:
> > Hi Steffen,
> >
> > On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
> > > +/**
> > > + * of_get_display_timings - parse all display_timing entries from a device_node
> > > + * @np: device_node with the subnodes
> > > + **/
> > > +struct display_timings *of_get_display_timings(const struct device_node *np)
> > > +{
> > > + struct device_node *timings_np;
> > > + struct device_node *entry;
> > > + struct device_node *native_mode;
> > > + struct display_timings *disp;
> > > +
> > > + if (!np) {
> > > + pr_err("%s: no devicenode given\n", __func__);
> > > + return NULL;
> > > + }
> > > +
> > > + timings_np = of_find_node_by_name(np, "display-timings");
> >
> > I get below build warnings on this line
> > drivers/video/of_display_timing.c: In function 'of_get_display_timings':
> > drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of 'of_find_node_by_name' discards qualifiers from pointer target type
> > include/linux/of.h:167:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> >
> > > + * of_display_timings_exists - check if a display-timings node is provided
> > > + * @np: device_node with the timing
> > > + **/
> > > +int of_display_timings_exists(const struct device_node *np)
> > > +{
> > > + struct device_node *timings_np;
> > > +
> > > + if (!np)
> > > + return -EINVAL;
> > > +
> > > + timings_np = of_parse_phandle(np, "display-timings", 0);
> >
> > Also here:
> > drivers/video/of_display_timing.c: In function 'of_display_timings_exists':
> > drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of 'of_parse_phandle' discards qualifiers from pointer target type
> > include/linux/of.h:258:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> >
>
> The warnings are because the of-functions do not use const pointers where they
> should. I had two options: don't use const pointers even if they should be and
> have no warnings or use const pointers and have a correct API. (Third option:
> send patches for of-functions). I chose the second option.
Maybe a better approach would be a combination of 1 and 3: don't use
const pointers for struct device_node for now and bring the issue up
with the OF maintainers, possibly with patches attached that fix the
problematic functions.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-11-21 11:58 UTC (permalink / raw)
To: Leela Krishna Amudala
Cc: Manjunathappa, Prakash, devicetree-discuss@lists.ozlabs.org,
linux-fbdev@vger.kernel.org, David Airlie,
Florian Tobias Schandinat, dri-devel@lists.freedesktop.org,
Valkeinen, Tomi, Laurent Pinchart, kernel@pengutronix.de,
Guennady Liakhovetski, linux-media@vger.kernel.org
In-Reply-To: <CAL1wa8dQ4QL0SzbXdo8nogBfBjQ8GpaJ134v6zu_iMkWQeXefA@mail.gmail.com>
Hi!
On Wed, Nov 21, 2012 at 04:39:01PM +0530, Leela Krishna Amudala wrote:
> Yes,
> Even I got the same build error.
> later I fixed it by including "#include <linux/mxsfb.h>"
>
> Best Wishes,
> Leela Krishna.
>
> On Wed, Nov 21, 2012 at 3:39 PM, Manjunathappa, Prakash
> <prakash.pm@ti.com> wrote:
> > Hi Steffen,
> >
> > I am trying to add DT support for da8xx-fb driver on top of your patches.
> > Encountered below build error. Sorry for reporting it late.
> >
> > On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote:
> >> Add a function to convert from the generic videomode to a fb_videomode.
> >>
> >> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> >> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> >> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> >> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> >> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> ---
> >> drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >> include/linux/fb.h | 6 ++++++
> >> 2 files changed, 52 insertions(+)
> >>
> >> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> >> index cef6557..c1939a6 100644
> >> --- a/drivers/video/fbmon.c
> >> +++ b/drivers/video/fbmon.c
> >> @@ -31,6 +31,7 @@
> >> #include <linux/pci.h>
> >> #include <linux/slab.h>
> >> #include <video/edid.h>
> >> +#include <linux/videomode.h>
> >> #ifdef CONFIG_PPC_OF
> >> #include <asm/prom.h>
> >> #include <asm/pci-bridge.h>
> >> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
> >> kfree(timings);
> >> return err;
> >> }
> >> +
> >> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> >> +int fb_videomode_from_videomode(const struct videomode *vm,
> >> + struct fb_videomode *fbmode)
> >> +{
> >> + unsigned int htotal, vtotal;
> >> +
> >> + fbmode->xres = vm->hactive;
> >> + fbmode->left_margin = vm->hback_porch;
> >> + fbmode->right_margin = vm->hfront_porch;
> >> + fbmode->hsync_len = vm->hsync_len;
> >> +
> >> + fbmode->yres = vm->vactive;
> >> + fbmode->upper_margin = vm->vback_porch;
> >> + fbmode->lower_margin = vm->vfront_porch;
> >> + fbmode->vsync_len = vm->vsync_len;
> >> +
> >> + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
> >> +
> >> + fbmode->sync = 0;
> >> + fbmode->vmode = 0;
> >> + if (vm->hah)
> >> + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> >> + if (vm->vah)
> >> + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> >> + if (vm->interlaced)
> >> + fbmode->vmode |= FB_VMODE_INTERLACED;
> >> + if (vm->doublescan)
> >> + fbmode->vmode |= FB_VMODE_DOUBLE;
> >> + if (vm->de)
> >> + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
> >
> > "FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting
> > build error on this. Please let me know if I am missing something.
> >
> > Thanks,
> > Prakash
> >
I compile-tested the series and didn't have that error. But obviously I should
have. As this is a mxsfs flag, I will throw it out.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 298/493] video: remove use of __devinitdata
From: Russell King - ARM Linux @ 2012-11-21 12:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353349642-3677-298-git-send-email-wfp5p@virginia.edu>
On Mon, Nov 19, 2012 at 01:24:07PM -0500, Bill Pemberton wrote:
> drivers/video/acornfb.c | 8 +++---
> drivers/video/cyber2000fb.c | 2 +-
> drivers/video/sa1100fb.c | 2 +-
For these three,
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21 12:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121121114018.GA31576@avionic-0098.adnet.avionic-design.de>
[-- Attachment #1: Type: text/plain, Size: 5646 bytes --]
On 2012-11-21 13:40, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 01:06:03PM +0200, Tomi Valkeinen wrote:
(sorry for bouncing back and forth with my private and my @ti addresses.
I can't find an option in thunderbird to only use one sender address,
and I always forget to change it when responding...)
>> My suggestion would be to go forward with an in-driver solution, and
>> look at the DT based solution later if we are seeing an increasing bloat
>> in the drivers.
>
> Assuming we go with your approach, what's the plan? We're actually
> facing this problem right now for Tegra. Basically we have a DRM driver
> that can drive the panel, but we're still missing a way to hook up the
> backlight and panel enabling code. So we effectively can't support any
> of the LVDS devices out there without this series.
Could you describe the hardware setup you have related to the LCD and
backlight? Is it a public board with public schematics?
I've understood that you don't have anything special in your board, just
an LCD and a backlight, and the power sequences are related to powering
up the LCD and the backlight, without anything board specific. If so,
there's no need for board specific code, but just improving the panel
and backlight drivers to support the models you use.
> As I understand it, what you propose is similar to what ASoC does. For a
> specific board, you'd have to write a driver, presumably for the new
> panel/display framework, that provides code to power the panel on and
> off. That means we'll have to have a driver for each panel out there
> basically, or we'd need to write generic drivers that can be configured
> to some degree (via platform data or DT). This is similar to how ASoC
> works, where we have a driver that provides support for a specific codec
> connected to the Tegra SoC. For the display framework things could be
> done in a similar way I suppose, so that Tegra could have one display
> driver to handle all aspects of powering on and off the various panels
> for the various boards out there.
I think we should only need the board drivers for very special cases. If
there's just a panel and a backlight, without any special dynamic muxing
or other trickery needed, I don't see a need for a board driver. I
presume this is the case for most of the boards.
> Obviously, a lot of the code will be similar for other SoCs, but maybe
> that's just the way things are if we choose that approach. There's also
> the potential for factoring out large chunks of common code later on
> once we start to see common patterns.
>
> One thing that's not very clear is how the backlight subsystem should be
> wired up with the display framework. I have a patch on top of the Tegra
> DRM driver which adds some ad-hoc display support using this power
> sequences series and the pwm-backlight.
I think that's a separate issue: how to associate the lcd device and
backlight device together. I don't have a clear answer to this.
There are many ways the backlight may be handled. In some cases the
panel and the backlight are truly independent, and you can use the other
without using the other (not very practical, though =).
But then with some LCDs the backlight may be controlled by sending
commands to the panel, and in this case the two may be quite linked.
Changing the backlight requires the panel driver to be up and running,
and sometimes the sending the backlight commands may need to be (say,
DSI display, with backlight commands going over the DSI bus).
So my feeling is that the panel driver should know about the related
backlight device. In the first case the panel driver would just call
enable/disable in the backlight device when the panel is turned on.
In the second case of the DSI panel... I'm not sure. I've implemented it
so that the panel driver creates the backlight device, and implements
the backlight callbacks. It then sends the DSI commands from those
callbacks.
> From reading the proposal for the panel/display framework, it sounds
> like a lot more is planned than just enabling or disabling panels, but
> it also seems like a lot of code needs to be written to support things
> like DSI, DBI or other control busses.
>
> At least for Tegra, and I think the same holds for a wide variety of
> other SoCs, dumb panels would be enough for a start. In the interest of
> getting a working solution for those setups, maybe we can start small
> and add just enough framework to register dumb panel drivers to along
> with code to wire up a backlight to light up the display. Then we could
> possibly still make it to have a proper solution to support the various
> LVDS panels for Tegra with 3.9.
Yes, we (Laurent and me) both agree that we should start simple.
However, the common panel framework is not strictly needed for this. I'm
not sure of the current architecture for Tegra, but for OMAP we already
have panel drivers (omap specific ones, though). The panel drivers may
support multiple models, (for example,
drivers/video/omap2/displays/panel-generic-dpi.c).
I don't see any problem with adding small Tegra specific panel drivers
for the time being, with the intention of converting to common panel
framework when that's available.
Of course, the DT side is an issue. If you now create DT bindings for a
temporary model, and need to change it again later, you'll have some
headaches trying managing that without breaking the old bindings... This
is why I haven't pushed DT bindings for OMAP, as I know I have to change
them in the near future.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Tomi Valkeinen @ 2012-11-21 12:22 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
David Airlie, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
Philipp Zabel, Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353426896-6045-3-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3641 bytes --]
On 2012-11-20 17:54, Steffen Trumtrar wrote:
> +timings subnode
> +---------------
> +
> +required properties:
> + - hactive, vactive: Display resolution
> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> + in pixels
> + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> + lines
> + - clock-frequency: display clock in Hz
> +
> +optional properties:
> + - hsync-active: Hsync pulse is active low/high/ignored
> + - vsync-active: Vsync pulse is active low/high/ignored
> + - de-active: Data-Enable pulse is active low/high/ignored
> + - pixelclk-inverted: pixelclock is inverted/non-inverted/ignored
Inverted related to what? And isn't this a bool? Pixel clock is either
normal (whatever that is), or inverted. It can't be "not used".
I guess normal case is "pixel data is driven on the rising edge of pixel
clock"? If that's common knowledge, I guess it doesn't need to be
mentioned. But I always have to verify from the documentation what
"normal" means on this particular panel/soc =).
> + - interlaced (bool)
> + - doublescan (bool)
> +
> +All the optional properties that are not bool follow the following logic:
> + <1>: high active
> + <0>: low active
> + omitted: not used on hardware
Perhaps it's obvious, but no harm being explicit: mention that the bool
properties are off is omitted.
And I didn't read the rest of the patches yet, so perhaps this is
already correct, but as I think this framework is usable without DT
also, the meanings of the fields in the structs should be explained in
the header files also in case they are not obvious.
> +Example:
> +
> + display-timings {
> + native-mode = <&timing0>;
> + timing0: 1920p24 {
This should still be 1080p24, not 1920p24 =).
> + /* 1920x1080p24 */
> + clock-frequency = <52000000>;
> + hactive = <1920>;
> + vactive = <1080>;
> + hfront-porch = <25>;
> + hback-porch = <25>;
> + hsync-len = <25>;
> + vback-porch = <2>;
> + vfront-porch = <2>;
> + vsync-len = <2>;
> + hsync-active = <1>;
> + };
> + };
> +
> diff --git a/include/linux/of_display_timings.h b/include/linux/of_display_timings.h
> new file mode 100644
> index 0000000..2b4fa0a
> --- /dev/null
> +++ b/include/linux/of_display_timings.h
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * display timings of helpers
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
> +#define __LINUX_OF_DISPLAY_TIMINGS_H
> +
> +#include <linux/display_timing.h>
> +#include <linux/of.h>
No need to include these, just add "struct ...;".
> +#define OF_USE_NATIVE_MODE -1
> +
> +struct display_timings *of_get_display_timings(const struct device_node *np);
> +int of_display_timings_exists(const struct device_node *np);
> +
> +#endif
> diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
> new file mode 100644
> index 0000000..4de5fcc
> --- /dev/null
> +++ b/include/linux/of_videomode.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> + *
> + * videomode of-helpers
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#ifndef __LINUX_OF_VIDEOMODE_H
> +#define __LINUX_OF_VIDEOMODE_H
> +
> +#include <linux/videomode.h>
> +#include <linux/of.h>
Same here.
> +int of_get_videomode(const struct device_node *np, struct videomode *vm,
> + int index);
> +
> +#endif /* __LINUX_OF_VIDEOMODE_H */
>
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH v12 3/6] fbmon: add videomode helpers
From: Tomi Valkeinen @ 2012-11-21 12:27 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Stephen Warren, kernel, Florian Tobias Schandinat,
David Airlie
In-Reply-To: <1353426896-6045-4-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 969 bytes --]
On 2012-11-20 17:54, Steffen Trumtrar wrote:
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..920cbe3 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -14,6 +14,7 @@
> #include <linux/backlight.h>
> #include <linux/slab.h>
> #include <asm/io.h>
> +#include <linux/videomode.h>
No need for this, just add "struct xxx;".
> struct vm_area_struct;
> struct fb_info;
> @@ -714,6 +715,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
> extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
> extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> + struct fb_videomode *fbmode);
> +#endif
> +
> /* drivers/video/modedb.c */
> #define VESA_MODEDB_SIZE 34
> extern void fb_var_to_videomode(struct fb_videomode *mode,
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH v12 4/6] fbmon: add of_videomode helpers
From: Tomi Valkeinen @ 2012-11-21 12:49 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353426896-6045-5-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]
On 2012-11-20 17:54, Steffen Trumtrar wrote:
> Add helper to get fb_videomode from devicetree.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/video/fbmon.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> include/linux/fb.h | 7 +++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 920cbe3..41b5e49 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -15,6 +15,8 @@
> #include <linux/slab.h>
> #include <asm/io.h>
> #include <linux/videomode.h>
> +#include <linux/of.h>
> +#include <linux/of_videomode.h>
Guess what? =)
To be honest, I don't know what the general opinion is about including
header files from header files. But I always leave them out if they are
not strictly needed.
> struct vm_area_struct;
> struct fb_info;
> @@ -715,6 +717,11 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
> extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
> extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>
> +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
> +extern int of_get_fb_videomode(const struct device_node *np,
> + struct fb_videomode *fb,
> + unsigned int index);
> +#endif
> #if IS_ENABLED(CONFIG_VIDEOMODE)
> extern int fb_videomode_from_videomode(const struct videomode *vm,
> struct fb_videomode *fbmode);
Do you really need these #ifs in the header files? They do make it look
a bit messy. If somebody uses the functions and CONFIG_VIDEOMODE is not
enabled, he'll get a linker error anyway.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH v12 5/6] drm_modes: add videomode helpers
From: Tomi Valkeinen @ 2012-11-21 12:50 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Stephen Warren, kernel, Florian Tobias Schandinat,
David Airlie
In-Reply-To: <1353426896-6045-6-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]
On 2012-11-20 17:54, Steffen Trumtrar wrote:
> Add conversion from videomode to drm_display_mode
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/drm_modes.c | 37 +++++++++++++++++++++++++++++++++++++
> include/drm/drmP.h | 6 ++++++
> 2 files changed, 43 insertions(+)
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 3fd8280..de2f6cf 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -56,6 +56,7 @@
> #include <linux/cdev.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> +#include <linux/videomode.h>
> #if defined(__alpha__) || defined(__powerpc__)
> #include <asm/pgtable.h> /* For pte_wrprotect */
> #endif
> @@ -1454,6 +1455,11 @@ extern struct drm_display_mode *
> drm_mode_create_from_cmdline_mode(struct drm_device *dev,
> struct drm_cmdline_mode *cmd);
>
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +extern int drm_display_mode_from_videomode(const struct videomode *vm,
> + struct drm_display_mode *dmode);
> +#endif
> +
> /* Modesetting support */
> extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
> extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
>
And the same comments apply to this header file.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH v12 6/6] drm_modes: add of_videomode helpers
From: Tomi Valkeinen @ 2012-11-21 12:52 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353426896-6045-7-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]
On 2012-11-20 17:54, Steffen Trumtrar wrote:
> Add helper to get drm_display_mode from devicetree.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/drm_modes.c | 35 ++++++++++++++++++++++++++++++++++-
> include/drm/drmP.h | 6 ++++++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index de2f6cf..377280f 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -56,6 +56,7 @@
> #include <linux/cdev.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
> #include <linux/videomode.h>
> #if defined(__alpha__) || defined(__powerpc__)
> #include <asm/pgtable.h> /* For pte_wrprotect */
> @@ -1459,6 +1460,11 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
> extern int drm_display_mode_from_videomode(const struct videomode *vm,
> struct drm_display_mode *dmode);
> #endif
> +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
> +extern int of_get_drm_display_mode(const struct device_node *np,
> + struct drm_display_mode *dmode,
> + unsigned int index);
> +#endif
>
> /* Modesetting support */
> extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
And the same comments here also.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-11-21 13:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50ACC341.3090204@ti.com>
[-- Attachment #1: Type: text/plain, Size: 7937 bytes --]
On Wed, Nov 21, 2012 at 02:04:17PM +0200, Tomi Valkeinen wrote:
> On 2012-11-21 13:40, Thierry Reding wrote:
> > On Wed, Nov 21, 2012 at 01:06:03PM +0200, Tomi Valkeinen wrote:
>
> (sorry for bouncing back and forth with my private and my @ti addresses.
> I can't find an option in thunderbird to only use one sender address,
> and I always forget to change it when responding...)
>
> >> My suggestion would be to go forward with an in-driver solution, and
> >> look at the DT based solution later if we are seeing an increasing bloat
> >> in the drivers.
> >
> > Assuming we go with your approach, what's the plan? We're actually
> > facing this problem right now for Tegra. Basically we have a DRM driver
> > that can drive the panel, but we're still missing a way to hook up the
> > backlight and panel enabling code. So we effectively can't support any
> > of the LVDS devices out there without this series.
>
> Could you describe the hardware setup you have related to the LCD and
> backlight? Is it a public board with public schematics?
I don't think any of the schematics are public. The Tamonten Evaluation
Carrier is available publicly from our website and the schematics are
available on demand as well. If required I can probably arrange to send
you a copy.
> I've understood that you don't have anything special in your board, just
> an LCD and a backlight, and the power sequences are related to powering
> up the LCD and the backlight, without anything board specific. If so,
> there's no need for board specific code, but just improving the panel
> and backlight drivers to support the models you use.
Correct. Basically we have two GPIOs that each enable the panel or the
backlight respectively and one PWM to control the brightness. Are the
panel drivers that you refer to those in drivers/video? I'm not sure if
adding more ad-hoc drivers there just to move them to a generic
framework in the next cycle is a good idea. I'd rather spend some time
on helping to get the framework right and have drivers for that instead.
From what I understand by looking at the OMAP display drivers, they also
provide the timings for the displays. Steffen's videomode helpers can be
used to represent these easily in DT, but I suppose if all of those per-
panel specifics are represented in the drivers then that won't be needed
anymore either.
> > As I understand it, what you propose is similar to what ASoC does. For a
> > specific board, you'd have to write a driver, presumably for the new
> > panel/display framework, that provides code to power the panel on and
> > off. That means we'll have to have a driver for each panel out there
> > basically, or we'd need to write generic drivers that can be configured
> > to some degree (via platform data or DT). This is similar to how ASoC
> > works, where we have a driver that provides support for a specific codec
> > connected to the Tegra SoC. For the display framework things could be
> > done in a similar way I suppose, so that Tegra could have one display
> > driver to handle all aspects of powering on and off the various panels
> > for the various boards out there.
>
> I think we should only need the board drivers for very special cases. If
> there's just a panel and a backlight, without any special dynamic muxing
> or other trickery needed, I don't see a need for a board driver. I
> presume this is the case for most of the boards.
For Tegra ASoC, the way to provide for this is to allow a specific board
to introduce a separate compatible value to enable per-board quirks or
special handling if it cannot be supported by the generic driver and
configuration mechanisms.
> > Obviously, a lot of the code will be similar for other SoCs, but maybe
> > that's just the way things are if we choose that approach. There's also
> > the potential for factoring out large chunks of common code later on
> > once we start to see common patterns.
> >
> > One thing that's not very clear is how the backlight subsystem should be
> > wired up with the display framework. I have a patch on top of the Tegra
> > DRM driver which adds some ad-hoc display support using this power
> > sequences series and the pwm-backlight.
>
> I think that's a separate issue: how to associate the lcd device and
> backlight device together. I don't have a clear answer to this.
>
> There are many ways the backlight may be handled. In some cases the
> panel and the backlight are truly independent, and you can use the other
> without using the other (not very practical, though =).
At least for DT I think we can easily wire that up. I've actually posted
a patch recently that does so. I think in most cases it makes sense to
control them together, such as on DPMS changes, where you really want to
turn both the backlight and the LCD off, independent of how they are
tied together.
> But then with some LCDs the backlight may be controlled by sending
> commands to the panel, and in this case the two may be quite linked.
> Changing the backlight requires the panel driver to be up and running,
> and sometimes the sending the backlight commands may need to be (say,
> DSI display, with backlight commands going over the DSI bus).
>
> So my feeling is that the panel driver should know about the related
> backlight device. In the first case the panel driver would just call
> enable/disable in the backlight device when the panel is turned on.
Exactly.
> In the second case of the DSI panel... I'm not sure. I've implemented it
> so that the panel driver creates the backlight device, and implements
> the backlight callbacks. It then sends the DSI commands from those
> callbacks.
That certainly sounds like the right approach to me.
> > From reading the proposal for the panel/display framework, it sounds
> > like a lot more is planned than just enabling or disabling panels, but
> > it also seems like a lot of code needs to be written to support things
> > like DSI, DBI or other control busses.
> >
> > At least for Tegra, and I think the same holds for a wide variety of
> > other SoCs, dumb panels would be enough for a start. In the interest of
> > getting a working solution for those setups, maybe we can start small
> > and add just enough framework to register dumb panel drivers to along
> > with code to wire up a backlight to light up the display. Then we could
> > possibly still make it to have a proper solution to support the various
> > LVDS panels for Tegra with 3.9.
>
> Yes, we (Laurent and me) both agree that we should start simple.
>
> However, the common panel framework is not strictly needed for this. I'm
> not sure of the current architecture for Tegra, but for OMAP we already
> have panel drivers (omap specific ones, though). The panel drivers may
> support multiple models, (for example,
> drivers/video/omap2/displays/panel-generic-dpi.c).
>
> I don't see any problem with adding small Tegra specific panel drivers
> for the time being, with the intention of converting to common panel
> framework when that's available.
I can take a look at how such a driver could be implemented, but again,
I'm a bit reluctant to add something ad-hoc now when maybe we can have
it supported in a proper framework not too far away in the future.
> Of course, the DT side is an issue. If you now create DT bindings for a
> temporary model, and need to change it again later, you'll have some
> headaches trying managing that without breaking the old bindings... This
> is why I haven't pushed DT bindings for OMAP, as I know I have to change
> them in the near future.
We're already keeping back on this and none of the patches that define
the bindings have been merged yet. Although bindings have been known to
change every once in a while even for code that is already in mainline.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v12 4/6] fbmon: add of_videomode helpers
From: Laurent Pinchart @ 2012-11-21 13:08 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Steffen Trumtrar, devicetree-discuss, Rob Herring, linux-fbdev,
dri-devel, Thierry Reding, Guennady Liakhovetski, linux-media,
Stephen Warren, kernel, Florian Tobias Schandinat, David Airlie
In-Reply-To: <50ACCDDA.2070606@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2230 bytes --]
Hi Tomi,
On Wednesday 21 November 2012 14:49:30 Tomi Valkeinen wrote:
> On 2012-11-20 17:54, Steffen Trumtrar wrote:
> > Add helper to get fb_videomode from devicetree.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> > Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> > Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> > Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > drivers/video/fbmon.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> > include/linux/fb.h | 7 +++++++
> > 2 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > index 920cbe3..41b5e49 100644
> > --- a/include/linux/fb.h
> > +++ b/include/linux/fb.h
> > @@ -15,6 +15,8 @@
> >
> > #include <linux/slab.h>
> > #include <asm/io.h>
> > #include <linux/videomode.h>
> >
> > +#include <linux/of.h>
> > +#include <linux/of_videomode.h>
>
> Guess what? =)
>
> To be honest, I don't know what the general opinion is about including
> header files from header files. But I always leave them out if they are
> not strictly needed.
I agree, I favor structure declaration as well when possible.
> > struct vm_area_struct;
> > struct fb_info;
> >
> > @@ -715,6 +717,11 @@ extern void fb_destroy_modedb(struct fb_videomode
> > *modedb);>
> > extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int
> > rb); extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> >
> > +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
> > +extern int of_get_fb_videomode(const struct device_node *np,
> > + struct fb_videomode *fb,
> > + unsigned int index);
> > +#endif
> >
> > #if IS_ENABLED(CONFIG_VIDEOMODE)
> > extern int fb_videomode_from_videomode(const struct videomode *vm,
> >
> > struct fb_videomode *fbmode);
>
> Do you really need these #ifs in the header files? They do make it look
> a bit messy. If somebody uses the functions and CONFIG_VIDEOMODE is not
> enabled, he'll get a linker error anyway.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH v12 0/6] of: add display helper
From: Laurent Pinchart @ 2012-11-21 13:18 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: Thierry Reding, Robert Schwebel, devicetree-discuss, Rob Herring,
linux-fbdev, dri-devel, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel, Florian Tobias Schandinat,
David Airlie
In-Reply-To: <20121121082822.GB14013@pengutronix.de>
On Wednesday 21 November 2012 09:28:22 Steffen Trumtrar wrote:
> On Tue, Nov 20, 2012 at 08:35:31PM +0100, Thierry Reding wrote:
> > On Tue, Nov 20, 2012 at 07:11:29PM +0100, Robert Schwebel wrote:
> > > On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> > > > On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > > > > Hi!
> > > > >
> > > > > Changes since v11:
> > > > > - make pointers const where applicable
> > > > > - add reviewed-by Laurent Pinchart
> > > >
> > > > Looks good to me.
> > > >
> > > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Through which tree do you plan to push this ?
> > >
> > > We have no idea yet, and none of the people on Cc: have volunteered
> > > so far... what do you think?
> >
> > The customary approach would be to take the patches through separate
> > trees, but I think this particular series is sufficiently interwoven to
> > warrant taking them all through one tree. However the least that we
> > should do is collect Acked-by's from the other tree maintainers.
> >
> > Given that most of the patches modify files in drivers/video, Florian's
> > fbdev tree would be the most obvious candidate, right? If Florian agrees
> > to take the patches, all we would need is David's Acked-by.
> >
> > How does that sound?
> >
> > Thierry
>
> Hi!
>
> That is exactly the way I thought this could happen.
Sounds good to me as well.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21 13:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121121130039.GA12191@avionic-0098.adnet.avionic-design.de>
[-- Attachment #1: Type: text/plain, Size: 5456 bytes --]
On 2012-11-21 15:00, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 02:04:17PM +0200, Tomi Valkeinen wrote:
>> On 2012-11-21 13:40, Thierry Reding wrote:
>>> On Wed, Nov 21, 2012 at 01:06:03PM +0200, Tomi Valkeinen wrote:
>>
>> (sorry for bouncing back and forth with my private and my @ti addresses.
>> I can't find an option in thunderbird to only use one sender address,
>> and I always forget to change it when responding...)
>>
>>>> My suggestion would be to go forward with an in-driver solution, and
>>>> look at the DT based solution later if we are seeing an increasing bloat
>>>> in the drivers.
>>>
>>> Assuming we go with your approach, what's the plan? We're actually
>>> facing this problem right now for Tegra. Basically we have a DRM driver
>>> that can drive the panel, but we're still missing a way to hook up the
>>> backlight and panel enabling code. So we effectively can't support any
>>> of the LVDS devices out there without this series.
>>
>> Could you describe the hardware setup you have related to the LCD and
>> backlight? Is it a public board with public schematics?
>
> I don't think any of the schematics are public. The Tamonten Evaluation
> Carrier is available publicly from our website and the schematics are
> available on demand as well. If required I can probably arrange to send
> you a copy.
No need, I think your answer below is enough.
>> I've understood that you don't have anything special in your board, just
>> an LCD and a backlight, and the power sequences are related to powering
>> up the LCD and the backlight, without anything board specific. If so,
>> there's no need for board specific code, but just improving the panel
>> and backlight drivers to support the models you use.
>
> Correct. Basically we have two GPIOs that each enable the panel or the
> backlight respectively and one PWM to control the brightness. Are the
The panel GPIO goes to the panel hardware device, and enables the panel?
And similarly for the PWM GPIO, it goes to the PWM hardware device? Just
making sure there are no other components involved.
> panel drivers that you refer to those in drivers/video? I'm not sure if
> adding more ad-hoc drivers there just to move them to a generic
> framework in the next cycle is a good idea. I'd rather spend some time
> on helping to get the framework right and have drivers for that instead.
We have panel drivers for omap in drivers/video/omap2/displays/. I'm not
sure if other platforms have their own versions of panel drivers, but
probably adding a simple panel driver system for a platform would not be
too difficult. It could even be quite hardcoded, i.e. embedded directly
into the display subsystem driver, just to get something working until
the common panel framework is available.
Yes, I agree it's not good idea to add more platform specific panel
drivers. But it's unclear when CPF will be merged, so if you need to get
the panel working now, I don't see a simple ad-hoc driver as too
horrible. But, of course, I'm not the one making the decision whether to
merge or not =).
> From what I understand by looking at the OMAP display drivers, they also
> provide the timings for the displays. Steffen's videomode helpers can be
> used to represent these easily in DT, but I suppose if all of those per-
Right. Note that I didn't present omap panel drivers as perfect
examples, just examples =).
> panel specifics are represented in the drivers then that won't be needed
> anymore either.
Yes, for most panels with just one native mode and nothing else, the
panel driver can contain the timings.
However, this subject has been discussed earlier a few times. If the
panel in question doesn't need any special power-on/off sequences, just,
perhaps, one gpio or such, we could still use DT video modes. This would
simplify the cases where you have lots of different very simple panels.
Obviously the same questions apply to DT video modes than to the power
sequences, and while I do think it's better to handle the timings inside
the driver, I'm not too much against video timings in DT. The reason
being that the video modes are quite clear, simple and stable data,
versus the much more complex and open-ended power sequences.
>> I don't see any problem with adding small Tegra specific panel drivers
>> for the time being, with the intention of converting to common panel
>> framework when that's available.
>
> I can take a look at how such a driver could be implemented, but again,
Don't look at the mainline omap panel drivers, at least not too closely
=). They contain hackery that will be cleaned up with CPF.
I think there are two methods to implements simple panel drivers:
The hardcoded one, where the display subsystem driver manages a few
different panel models. This is obviously not very expandable or
"correct", but should probably work just fine for a few models, until
CPF is usable.
Something like CPF will have: have the panel device/driver as a platform
device/driver, which will register itself to the display subsystem. And
with "itself" I mean some kind of struct panel_entity, with a few ops
implemented by the panel driver.
Well, this goes a bit out of subject. If you want to discuss panel
drivers more, please start a new thread. Laurent's upcoming CPF v2
should give you good ideas what the model will be.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] OMAPFB: use dma_alloc instead of omap's vram
From: Jello huang @ 2012-11-21 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121120221456.GO18567@atomide.com>
HI Tomi,
we need one rank of cma to allocate the memory for driver in kernel
space .And the default CMA is for allocating memory frome usespace.So
if we allocate the memory from the
default CMA zone ,there maybe introduce fragmention to the default CMA
zone.The kernel space memory donot touch the memory from userspace
Jello Huang
On 21/11/2012, Tony Lindgren <tony@atomide.com> wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [121115 23:17]:
>>
>> I added your acks, and pushed:
>>
>> git://gitorious.org/linux-omap-dss2/linux.git 3.8/vram-conversion
>>
>> It's based on -rc4 as my other branches are based on that.
>
> OK thanks!
>
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Thanks
Jello Huang
^ permalink raw reply
* Re: efifb swapping Red/Blue color
From: Peter Jones @ 2012-11-21 14:38 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20121121001457.2672d788@neptune.home>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1391 bytes --]
On Wed, Nov 21, 2012 at 12:14:57AM +0100, Bruno Prémont wrote:
> On a MacBookAir2,1 with 9400M C79 nVidia GPU fbcon on efifb has
> red and blue colors swapped.
>
> Extract from kernel log:
> efifb: probing for efifb
> efifb: framebuffer at 0x80010000, mapped to 0xffffc90004580000, using 6400k, total 6400k
> efifb: mode is 1280x800x32, linelength92, pages=1
> efifb: scrolling: redraw
> efifb: Truecolor: size=8:8:8:8, shift$:0:8:16
>
> On another system with radeon GPU and externally connected DVI monitor
> colors are set properly.
>
> Probably efifb should be looking at something to determine if display
> is RGB or BGR...
Right - normally we get it from the firmware graphics driver, but on a
few generations of Intel Macs they gracefully omitted an implementation
of that part of the spec, which is why we have a DMI table that just has
best-effort entries.
So that on that hardware, it doesn't really give us anything to look
at. Though if the same machine comes with two different video cards
that are, by default, configured opposite ways, there may be something
we can do. Could show me the "lspci -v" for each of those devices (the
nvidia and the radeon)?
--
Peter
--
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: efifb swapping Red/Blue color
From: Bruno Prémont @ 2012-11-21 14:54 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20121121001457.2672d788@neptune.home>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 5021 bytes --]
Hi Peter,
On Wed, 21 November 2012 Peter Jones <pjones@redhat.com> wrote:
> On Wed, Nov 21, 2012 at 12:14:57AM +0100, Bruno Prémont wrote:
> > On a MacBookAir2,1 with 9400M C79 nVidia GPU fbcon on efifb has
> > red and blue colors swapped.
> >
> > Extract from kernel log:
> > efifb: probing for efifb
> > efifb: framebuffer at 0x80010000, mapped to 0xffffc90004580000, using 6400k, total 6400k
> > efifb: mode is 1280x800x32, linelength92, pages=1
> > efifb: scrolling: redraw
> > efifb: Truecolor: size=8:8:8:8, shift$:0:8:16
> >
> > On another system with radeon GPU and externally connected DVI monitor
> > colors are set properly.
> >
> > Probably efifb should be looking at something to determine if display
> > is RGB or BGR...
>
> Right - normally we get it from the firmware graphics driver, but on a
> few generations of Intel Macs they gracefully omitted an implementation
> of that part of the spec, which is why we have a DMI table that just has
> best-effort entries.
>
> So that on that hardware, it doesn't really give us anything to look
> at. Though if the same machine comes with two different video cards
> that are, by default, configured opposite ways, there may be something
> we can do. Could show me the "lspci -v" for each of those devices (the
> nvidia and the radeon)?
The system with Radeon is not the Mac (it's some Gigabyte mainboard that
has EFI support).
Here is lspci for the MacBook:
00:00.0 Host bridge [0600]: NVIDIA Corporation MCP79 Host Bridge [10de:0a82] (rev b1)
00:00.1 RAM memory [0500]: NVIDIA Corporation MCP79 Memory Controller [10de:0a88] (rev b1)
00:03.0 ISA bridge [0601]: NVIDIA Corporation MCP79 LPC Bridge [10de:0aaf] (rev b2)
00:03.1 RAM memory [0500]: NVIDIA Corporation MCP79 Memory Controller [10de:0aa4] (rev b1)
00:03.2 SMBus [0c05]: NVIDIA Corporation MCP79 SMBus [10de:0aa2] (rev b1)
00:03.3 RAM memory [0500]: NVIDIA Corporation MCP79 Memory Controller [10de:0a89] (rev b1)
00:03.4 RAM memory [0500]: NVIDIA Corporation Device [10de:0a98] (rev b1)
00:03.5 Co-processor [0b40]: NVIDIA Corporation MCP79 Co-processor [10de:0aa3] (rev b1)
00:04.0 USB controller [0c03]: NVIDIA Corporation MCP79 OHCI USB 1.1 Controller [10de:0aa5] (rev b1)
00:04.1 USB controller [0c03]: NVIDIA Corporation MCP79 EHCI USB 2.0 Controller [10de:0aa6] (rev b1)
00:06.0 USB controller [0c03]: NVIDIA Corporation MCP79 OHCI USB 1.1 Controller [10de:0aa7] (rev b1)
00:06.1 USB controller [0c03]: NVIDIA Corporation MCP79 EHCI USB 2.0 Controller [10de:0aa9] (rev b1)
00:08.0 Audio device [0403]: NVIDIA Corporation MCP79 High Definition Audio [10de:0ac0] (rev b1)
00:09.0 PCI bridge [0604]: NVIDIA Corporation MCP79 PCI Bridge [10de:0aab] (rev b1)
00:0b.0 SATA controller [0106]: NVIDIA Corporation MCP79 AHCI Controller [10de:0ab9] (rev b1)
00:10.0 PCI bridge [0604]: NVIDIA Corporation MCP79 PCI Express Bridge [10de:0aa0] (rev b1)
00:15.0 PCI bridge [0604]: NVIDIA Corporation MCP79 PCI Express Bridge [10de:0ac6] (rev b1)
02:00.0 VGA compatible controller [0300]: NVIDIA Corporation C79 [GeForce 9400M] [10de:0870] (rev b1)
03:00.0 Network controller [0280]: Broadcom Corporation BCM4321 802.11a/b/g/n [14e4:4328] (rev 05)
and DMI info:
# dmidecode 2.10
# SMBIOS entry point at 0x7fec7000
SMBIOS 2.4 present.
37 structures occupying 1888 bytes.
Table at 0x7FEC6000.
...
Handle 0x000E, DMI type 0, 24 bytes
BIOS Information
Vendor: Apple Inc.
Version: MBA21.88Z.0075.B03.0811141325
Release Date: 11/14/08
ROM Size: 4096 kB
Characteristics:
PCI is supported
BIOS is upgradeable
BIOS shadowing is allowed
Boot from CD is supported
Selectable boot is supported
ACPI is supported
IEEE 1394 boot is supported
Smart battery is supported
Function key-initiated network boot is supported
BIOS Revision: 0.1
Handle 0x000F, DMI type 1, 27 bytes
System Information
Manufacturer: Apple Inc.
Product Name: MacBookAir2,1
Version: 1.0
Serial Number: xxxxxxxxxxx
UUID: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
Wake-up Type: Power Switch
SKU Number: System SKU#
Family: MacBook
Handle 0x0010, DMI type 2, 16 bytes
Base Board Information
Manufacturer: Apple Inc.
Product Name: Mac-F42D88C8
Version: Not Specified
Serial Number: Base Board Serial#
Asset Tag: Base Board Asset Tag#
Features:
Board is a hosting board
Board is replaceable
Location In Chassis: Part Component
Chassis Handle: 0x0011
Type: Motherboard
Contained Object Handles: 0
Handle 0x0011, DMI type 3, 21 bytes
Chassis Information
Manufacturer: Apple Inc.
Type: Notebook
Lock: Not Present
Version: Mac-F42D88C8
Serial Number: xxxxxxxxxxx
Asset Tag: Asset Tag#
Boot-up State: Safe
Power Supply State: Safe
Thermal State: Other
Security Status: Other
OEM Information: 0x00000000
Height: Unspecified
Number Of Power Cords: Unspecified
Contained Elements: 0
...
Bruno
--
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: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-21 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50ACD7DC.5060405@ti.com>
Mmmm so maybe I am misinterpreting things, but it looks like we
have just buried the power sequences here, haven't we?
Alex.
On Wed, Nov 21, 2012 at 10:32 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 2012-11-21 15:00, Thierry Reding wrote:
>> On Wed, Nov 21, 2012 at 02:04:17PM +0200, Tomi Valkeinen wrote:
>>> On 2012-11-21 13:40, Thierry Reding wrote:
>>>> On Wed, Nov 21, 2012 at 01:06:03PM +0200, Tomi Valkeinen wrote:
>>>
>>> (sorry for bouncing back and forth with my private and my @ti addresses.
>>> I can't find an option in thunderbird to only use one sender address,
>>> and I always forget to change it when responding...)
>>>
>>>>> My suggestion would be to go forward with an in-driver solution, and
>>>>> look at the DT based solution later if we are seeing an increasing bloat
>>>>> in the drivers.
>>>>
>>>> Assuming we go with your approach, what's the plan? We're actually
>>>> facing this problem right now for Tegra. Basically we have a DRM driver
>>>> that can drive the panel, but we're still missing a way to hook up the
>>>> backlight and panel enabling code. So we effectively can't support any
>>>> of the LVDS devices out there without this series.
>>>
>>> Could you describe the hardware setup you have related to the LCD and
>>> backlight? Is it a public board with public schematics?
>>
>> I don't think any of the schematics are public. The Tamonten Evaluation
>> Carrier is available publicly from our website and the schematics are
>> available on demand as well. If required I can probably arrange to send
>> you a copy.
>
> No need, I think your answer below is enough.
>
>>> I've understood that you don't have anything special in your board, just
>>> an LCD and a backlight, and the power sequences are related to powering
>>> up the LCD and the backlight, without anything board specific. If so,
>>> there's no need for board specific code, but just improving the panel
>>> and backlight drivers to support the models you use.
>>
>> Correct. Basically we have two GPIOs that each enable the panel or the
>> backlight respectively and one PWM to control the brightness. Are the
>
> The panel GPIO goes to the panel hardware device, and enables the panel?
> And similarly for the PWM GPIO, it goes to the PWM hardware device? Just
> making sure there are no other components involved.
>
>> panel drivers that you refer to those in drivers/video? I'm not sure if
>> adding more ad-hoc drivers there just to move them to a generic
>> framework in the next cycle is a good idea. I'd rather spend some time
>> on helping to get the framework right and have drivers for that instead.
>
> We have panel drivers for omap in drivers/video/omap2/displays/. I'm not
> sure if other platforms have their own versions of panel drivers, but
> probably adding a simple panel driver system for a platform would not be
> too difficult. It could even be quite hardcoded, i.e. embedded directly
> into the display subsystem driver, just to get something working until
> the common panel framework is available.
>
> Yes, I agree it's not good idea to add more platform specific panel
> drivers. But it's unclear when CPF will be merged, so if you need to get
> the panel working now, I don't see a simple ad-hoc driver as too
> horrible. But, of course, I'm not the one making the decision whether to
> merge or not =).
>
>> From what I understand by looking at the OMAP display drivers, they also
>> provide the timings for the displays. Steffen's videomode helpers can be
>> used to represent these easily in DT, but I suppose if all of those per-
>
> Right. Note that I didn't present omap panel drivers as perfect
> examples, just examples =).
>
>> panel specifics are represented in the drivers then that won't be needed
>> anymore either.
>
> Yes, for most panels with just one native mode and nothing else, the
> panel driver can contain the timings.
>
> However, this subject has been discussed earlier a few times. If the
> panel in question doesn't need any special power-on/off sequences, just,
> perhaps, one gpio or such, we could still use DT video modes. This would
> simplify the cases where you have lots of different very simple panels.
>
> Obviously the same questions apply to DT video modes than to the power
> sequences, and while I do think it's better to handle the timings inside
> the driver, I'm not too much against video timings in DT. The reason
> being that the video modes are quite clear, simple and stable data,
> versus the much more complex and open-ended power sequences.
>
>>> I don't see any problem with adding small Tegra specific panel drivers
>>> for the time being, with the intention of converting to common panel
>>> framework when that's available.
>>
>> I can take a look at how such a driver could be implemented, but again,
>
> Don't look at the mainline omap panel drivers, at least not too closely
> =). They contain hackery that will be cleaned up with CPF.
>
> I think there are two methods to implements simple panel drivers:
>
> The hardcoded one, where the display subsystem driver manages a few
> different panel models. This is obviously not very expandable or
> "correct", but should probably work just fine for a few models, until
> CPF is usable.
>
> Something like CPF will have: have the panel device/driver as a platform
> device/driver, which will register itself to the display subsystem. And
> with "itself" I mean some kind of struct panel_entity, with a few ops
> implemented by the panel driver.
>
> Well, this goes a bit out of subject. If you want to discuss panel
> drivers more, please start a new thread. Laurent's upcoming CPF v2
> should give you good ideas what the model will be.
>
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Rob Herring @ 2012-11-21 15:03 UTC (permalink / raw)
To: Thierry Reding
Cc: Steffen Trumtrar, Manjunathappa, Prakash,
devicetree-discuss@lists.ozlabs.org, Philipp Zabel,
linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
Laurent Pinchart, Guennady Liakhovetski,
linux-media@vger.kernel.org, Valkeinen, Tomi, Stephen Warren,
kernel@pengutronix.de, Florian Tobias Schandinat, David Airlie,
Grant Likely
In-Reply-To: <20121121115236.GA8886@avionic-0098.adnet.avionic-design.de>
On 11/21/2012 05:52 AM, Thierry Reding wrote:
> On Wed, Nov 21, 2012 at 12:48:43PM +0100, Steffen Trumtrar wrote:
>> Hi!
>>
>> On Wed, Nov 21, 2012 at 10:12:43AM +0000, Manjunathappa, Prakash wrote:
>>> Hi Steffen,
>>>
>>> On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
>>>> +/**
>>>> + * of_get_display_timings - parse all display_timing entries from a device_node
>>>> + * @np: device_node with the subnodes
>>>> + **/
>>>> +struct display_timings *of_get_display_timings(const struct device_node *np)
>>>> +{
>>>> + struct device_node *timings_np;
>>>> + struct device_node *entry;
>>>> + struct device_node *native_mode;
>>>> + struct display_timings *disp;
>>>> +
>>>> + if (!np) {
>>>> + pr_err("%s: no devicenode given\n", __func__);
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + timings_np = of_find_node_by_name(np, "display-timings");
>>>
>>> I get below build warnings on this line
>>> drivers/video/of_display_timing.c: In function 'of_get_display_timings':
>>> drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of 'of_find_node_by_name' discards qualifiers from pointer target type
>>> include/linux/of.h:167:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
>>>
>>>> + * of_display_timings_exists - check if a display-timings node is provided
>>>> + * @np: device_node with the timing
>>>> + **/
>>>> +int of_display_timings_exists(const struct device_node *np)
>>>> +{
>>>> + struct device_node *timings_np;
>>>> +
>>>> + if (!np)
>>>> + return -EINVAL;
>>>> +
>>>> + timings_np = of_parse_phandle(np, "display-timings", 0);
>>>
>>> Also here:
>>> drivers/video/of_display_timing.c: In function 'of_display_timings_exists':
>>> drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of 'of_parse_phandle' discards qualifiers from pointer target type
>>> include/linux/of.h:258:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
>>>
>>
>> The warnings are because the of-functions do not use const pointers where they
>> should. I had two options: don't use const pointers even if they should be and
>> have no warnings or use const pointers and have a correct API. (Third option:
>> send patches for of-functions). I chose the second option.
>
> Maybe a better approach would be a combination of 1 and 3: don't use
> const pointers for struct device_node for now and bring the issue up
> with the OF maintainers, possibly with patches attached that fix the
> problematic functions.
Why does this need to be const? Since some DT functions increment
refcount the node, I'm not sure that making struct device_node const in
general is right thing to do. I do think it should be okay for
of_parse_phandle.
Rob
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-11-21 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAVeFuJofYwj926=3v0ApBCq_vNhCdC1PnzDc-B3d=LL_L40Uw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 660 bytes --]
On Thu, Nov 22, 2012 at 12:02:47AM +0900, Alexandre Courbot wrote:
> Mmmm so maybe I am misinterpreting things, but it looks like we
> have just buried the power sequences here, haven't we?
I don't think so. In fact I was just starting to think that maybe for
Tegra we could have a generic panel driver which used power sequences
to abstract away the pin differences for powering up the panel. Since
most likely that will be where the differences are there is a lot of
potential to factor things out into sequences.
Perhaps what we may want to postpone for now is the DT representation
since that's what Tomi and Grant seem to be mostly opposed to.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v12 2/6] video: add of helper for videomode
From: Thierry Reding @ 2012-11-21 15:13 UTC (permalink / raw)
To: Rob Herring
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, David Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
Manjunathappa, Prakash, Valkeinen, Tomi, Laurent Pinchart,
Philipp Zabel, Steffen Trumtrar, Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <50ACED4A.5040806-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3155 bytes --]
On Wed, Nov 21, 2012 at 09:03:38AM -0600, Rob Herring wrote:
> On 11/21/2012 05:52 AM, Thierry Reding wrote:
> > On Wed, Nov 21, 2012 at 12:48:43PM +0100, Steffen Trumtrar wrote:
> >> Hi!
> >>
> >> On Wed, Nov 21, 2012 at 10:12:43AM +0000, Manjunathappa, Prakash wrote:
> >>> Hi Steffen,
> >>>
> >>> On Tue, Nov 20, 2012 at 21:24:52, Steffen Trumtrar wrote:
> >>>> +/**
> >>>> + * of_get_display_timings - parse all display_timing entries from a device_node
> >>>> + * @np: device_node with the subnodes
> >>>> + **/
> >>>> +struct display_timings *of_get_display_timings(const struct device_node *np)
> >>>> +{
> >>>> + struct device_node *timings_np;
> >>>> + struct device_node *entry;
> >>>> + struct device_node *native_mode;
> >>>> + struct display_timings *disp;
> >>>> +
> >>>> + if (!np) {
> >>>> + pr_err("%s: no devicenode given\n", __func__);
> >>>> + return NULL;
> >>>> + }
> >>>> +
> >>>> + timings_np = of_find_node_by_name(np, "display-timings");
> >>>
> >>> I get below build warnings on this line
> >>> drivers/video/of_display_timing.c: In function 'of_get_display_timings':
> >>> drivers/video/of_display_timing.c:109:2: warning: passing argument 1 of 'of_find_node_by_name' discards qualifiers from pointer target type
> >>> include/linux/of.h:167:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> >>>
> >>>> + * of_display_timings_exists - check if a display-timings node is provided
> >>>> + * @np: device_node with the timing
> >>>> + **/
> >>>> +int of_display_timings_exists(const struct device_node *np)
> >>>> +{
> >>>> + struct device_node *timings_np;
> >>>> +
> >>>> + if (!np)
> >>>> + return -EINVAL;
> >>>> +
> >>>> + timings_np = of_parse_phandle(np, "display-timings", 0);
> >>>
> >>> Also here:
> >>> drivers/video/of_display_timing.c: In function 'of_display_timings_exists':
> >>> drivers/video/of_display_timing.c:209:2: warning: passing argument 1 of 'of_parse_phandle' discards qualifiers from pointer target type
> >>> include/linux/of.h:258:28: note: expected 'struct device_node *' but argument is of type 'const struct device_node *'
> >>>
> >>
> >> The warnings are because the of-functions do not use const pointers where they
> >> should. I had two options: don't use const pointers even if they should be and
> >> have no warnings or use const pointers and have a correct API. (Third option:
> >> send patches for of-functions). I chose the second option.
> >
> > Maybe a better approach would be a combination of 1 and 3: don't use
> > const pointers for struct device_node for now and bring the issue up
> > with the OF maintainers, possibly with patches attached that fix the
> > problematic functions.
>
> Why does this need to be const? Since some DT functions increment
> refcount the node, I'm not sure that making struct device_node const in
> general is right thing to do. I do think it should be okay for
> of_parse_phandle.
I wasn't proposing to do it everywhere but only where possible. If the
node is modified in some way then obviously it shouldn't be const.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ 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