From: Mark Zhang <nvmarkzhang@gmail.com>
To: Alexandre Courbot <gnurou@gmail.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Thierry Reding <thierry.reding@avionic-design.de>,
Mark Zhang <markz@nvidia.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [RFC 1/4] video: panel: add CLAA101WA01A panel support
Date: Thu, 31 Jan 2013 04:54:46 +0000 [thread overview]
Message-ID: <5109F916.8080404@gmail.com> (raw)
In-Reply-To: <CAAVeFuJf9Ww7g8Q9RAG9Na1SZq2-n2MMxcAEBek52ZVG7RpL7g@mail.gmail.com>
On 01/31/2013 12:24 PM, Alexandre Courbot wrote:
> On Thu, Jan 31, 2013 at 12:51 PM, Mark Zhang <nvmarkzhang@gmail.com> wrote:
>>> DDC access is a property of the display controller, not the panel
>>> itself. The panel might be hooked up to a display controller's DDC/I2C
>>> channel as the target, but it isn't the host/controller of the DDC/I2C
>>> channel. As such, placing the nvidia,ddc property into the display
>>> controller node makes sense.
>>>
>>
>> Yes, DC triggers the DDC access and is the host of the DDC/I2C channel.
>> So I think it's reasonable to put nvidia,ddc property into the display
>> controller node. But the video mode info in EDID which be fetched via
>> DDC is the property of the panel, so this info should be provided by
>> panel driver. Actually display controller cares about the video modes,
>> not the way to get these info. So I think it's better to do the whole
>> work like this:
>>
>> 1) display controller relied on CDF to call "display_entity_get_modes"
>> 2) panel driver calls the function which is hooked to display controller
>> to get the video modes via DDC.
>> 3) if the video modes can't be fetched via DDC, panel driver provides
>> the info(either by hard-coded or defined in DT) anyway
>
> This would require a callback dedicated to this in display_entity and
> would break its simple design.
We just need to add a function pointer param which can be used by panel
driver in "display_entity_get_modes", don't we?
>
>> Just like I said above, display controller should not query DDC
>> directly. Since CDF already defines these for us, display controller
>> should call CDF's functions, like: display_entity_get_modes,
>> display_entity_get_size... I think this is the key difference I wanna
>> talk about. And also in this way, display controller doesn't need to
>> care about this 2 steps logics, just call "display_entity_get_modes" is OK.
>
> Well the fact is that it *is* the display controller that queries DDC
> directly. The panel is just a bus here, and the EDID EEPROM could
> perfectly work without it. If you model what you described with DT
> nodes, I'm afraid you will end up with something both complex (with DC
> node referencing panel node and back again for the EDID bus) and that
> does not picture reality accurately.
Display controller don't know whether the panel has EDID EEPROM but the
panel driver knows. So why we need to make display controller queries
EDID blindly? Since panel driver knows about all "video modes/panel
size" stuffs, why not we let it handle all these things?
>
> Alex.
>
next prev parent reply other threads:[~2013-01-31 4:54 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-30 3:02 [RFC 0/4] Use the Common Display Framework in tegra-drm Alexandre Courbot
2013-01-30 3:02 ` [RFC 1/4] video: panel: add CLAA101WA01A panel support Alexandre Courbot
2013-01-30 7:20 ` Mark Zhang
[not found] ` <5108C9C1.1090707-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-30 7:27 ` Alex Courbot
[not found] ` <5108CB4F.7000103-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-30 7:48 ` Thierry Reding
2013-01-30 8:08 ` Mark Zhang
[not found] ` <20130130074852.GB17547-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
2013-01-30 8:28 ` Alex Courbot
2013-01-30 20:19 ` Stephen Warren
[not found] ` <51098064.7030902-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-01-31 3:51 ` Mark Zhang
[not found] ` <5109EA2A.8020204-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-31 4:24 ` Alexandre Courbot
2013-01-31 4:54 ` Mark Zhang [this message]
2013-01-31 6:36 ` Alexandre Courbot
[not found] ` <CAAVeFuL_a1aAEDCFdhjMzZG40QuK3dcZqsWqfVpwmQbZsfiHRg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-01-31 7:30 ` Mark Zhang
[not found] ` <510A1DAC.1070106-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-01-31 17:25 ` Stephen Warren
2013-01-31 17:20 ` Stephen Warren
[not found] ` <510AA7F8.7070000-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-02-01 4:19 ` Mark Zhang
[not found] ` <1359514939-15653-2-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-30 20:27 ` Stephen Warren
[not found] ` <51098229.7080508-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-01-31 4:14 ` Alexandre Courbot
[not found] ` <CAAVeFuJkJ4cftWvSvt1YJa6c48JyJPVTu=i18yMHptZMi3DAzg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-01-31 17:23 ` Stephen Warren
2013-01-30 20:30 ` Stephen Warren
2013-01-30 3:02 ` [RFC 2/4] tegra: ventana: add display and backlight DT nodes Alexandre Courbot
2013-01-30 3:02 ` [RFC 4/4] tegra: enable CDF and claa101 panel Alexandre Courbot
[not found] ` <1359514939-15653-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-30 3:02 ` [RFC 3/4] drm: tegra: use the Common Display Framework Alexandre Courbot
[not found] ` <1359514939-15653-4-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-30 6:50 ` Mark Zhang
2013-01-30 7:01 ` Alex Courbot
[not found] ` <5108C55C.30104-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-30 7:24 ` Thierry Reding
[not found] ` <20130130072406.GA17128-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
2013-01-30 7:30 ` Alex Courbot
2013-01-30 7:46 ` Mark Zhang
2013-01-30 7:40 ` [RFC 0/4] Use the Common Display Framework in tegra-drm Thierry Reding
[not found] ` <20130130074020.GA17547-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
2013-01-30 8:23 ` Alex Courbot
2013-01-30 8:38 ` Sascha Hauer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5109F916.8080404@gmail.com \
--to=nvmarkzhang@gmail.com \
--cc=gnurou@gmail.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=markz@nvidia.com \
--cc=swarren@wwwdotorg.org \
--cc=thierry.reding@avionic-design.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).