On 17/10/13 11:53, Thierry Reding wrote:
> I keep wondering why we absolutely must have compatibility between CDF
> and this simple panel binding. DT content is supposed to concern itself
> with the description of hardware only. What about the following isn't an
> accurate description of the panel hardware?
>
> panel: panel {
> compatible = "cptt,claa101wb01";
>
> power-supply = <&vdd_pnl_reg>;
> enable-gpios = <&gpio 90 0>;
>
> backlight = <&backlight>;
> };
>
> dsi-controller {
> panel = <&panel>;
> };
That's quite similar to how my current out-of-mainline OMAP DSS DT
bindings work. The difference is that I have a reference from the panel
node to the video source (dsi-controller), instead of the other way
around. I just find it more natural. It works the same way as, say,
regulators, gpios, etc.
However, one thing unclear is where the panel node should be. You seem
to have a DSI panel here. If the panel is truly not controlled in any
way, maybe having the panel as a normal platform device is ok. But if
the DSI panel is controlled with DSI messages, should it be a child of
the dsi-controller node, the same way i2c peripherals are children of
i2c master?
>>> +static const struct drm_display_mode auo_b101aw03_mode = {
>>> + .clock = 51450,
>>> + .hdisplay = 1024,
>>> + .hsync_start = 1024 + 156,
>>> + .hsync_end = 1024 + 156 + 8,
>>> + .htotal = 1024 + 156 + 8 + 156,
>>> + .vdisplay = 600,
>>> + .vsync_start = 600 + 16,
>>> + .vsync_end = 600 + 16 + 6,
>>> + .vtotal = 600 + 16 + 6 + 16,
>>> + .vrefresh = 60,
>>> +};
>>
>> Instead of hardcoding the modes in the driver, which would then require to be
>> updated for every new simple panel model (and we know there are lots of them),
>> why don't you specify the modes in the panel DT node ? The simple panel driver
>> would then become much more generic. It would also allow board designers to
>> tweak h/v sync timings depending on the system requirements.
>
> Sigh... we keep second-guessing ourselves. Back at the time when power
> sequences were designed (and NAKed at the last minute), we all decided
> that the right thing to do would be to use specific compatible values
> for individual panels, because that would allow us to encode the power
> sequencing within the driver. And when we already have the panel model
> encoded in the compatible value, we might just as well encode the mode
> within the driver for that panel. Otherwise we'll have to keep adding
> the same mode timings for every board that uses the same panel.
Oh, I didn't feel "we all decided that the right thing..." =).
> I do agree though that it might be useful to tweak the mode in case the
> default one doesn't work. How about we provide a means to override the
> mode encoded in the driver using one specified in the DT? That's
> obviously a backwards-compatible change, so it could be added if or when
> it becomes necessary.
This sounds good to me.
Although maybe it'd be good to have the driver compatible with something
like "generic-panel", so that you could have:
compatible = "foo,specific-panel", "generic-panel";
and if there's no need for any power/gpio setup for your board, you may
skip adding "foo,specific-panel" support to the panel driver. Later,
somebody else may need to implement fine grained power/gpio support for
"foo,specific-panel", and it would just work.
Maybe that would help us with the problem of adding support in the
driver for a hundred different simple panels each used only once on a
specific board.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
