[PATCH V6 0/8] drm/exynos: few patches to enhance bridge chip support

[Ajay kumar]

Hi Laurent,

Please find the latest series here:
http://www.spinics.net/lists/dri-devel/msg66740.html

On Wed, Sep 17, 2014 at 3:23 PM, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
> Hi Thierry,
>
> On Wednesday 30 July 2014 11:40:54 Thierry Reding wrote:
>> On Wed, Jul 30, 2014 at 11:54:00AM +0530, Ajay kumar wrote:
>> > On Tue, Jul 29, 2014 at 5:17 PM, Thierry Reding wrote:
>> > > On Tue, Jul 29, 2014 at 01:42:09PM +0200, Andreas Färber wrote:
>> > >> Am 29.07.2014 13:36, schrieb Thierry Reding:
>> > >> > On Tue, Jul 29, 2014 at 01:21:48PM +0200, Andreas Färber wrote:
>> > >> >> Am 28.07.2014 08:13, schrieb Ajay kumar:
>> > >> >>> On 7/27/14, Andreas Färber <afaerber at suse.de> wrote:
>> > >> >>>> Am 25.07.2014 21:22, schrieb Ajay Kumar:
>> > >> >>>>> This series is based on exynos-drm-next branch of Inki Dae's tree
>> > >> >>>>> at:
>> > >> >>>>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.
>> > >> >>>>> git
>> > >> >>>>>
>> > >> >>>>> I have tested this after adding few DT changes for
>> > >> >>>>> exynos5250-snow,
>> > >> >>>>> exynos5420-peach-pit and exynos5800-peach-pi boards.
>> > >> >>>>
>> > >> >>>> I'm trying to test this with a modified exynos5250-spring DT
>> > >>
>> > >> [...]
>> > >>
>> > >> >> Unfortunately the most I got on Spring with attached DT was a blank
>> > >> >> screen with a white horizontal line in the middle.
>> > >> >>
>> > >> >> Do I need to specify a specific panel model for Spring?
>> > >>
>> > >> [...]
>> > >>
>> > >> >> From 9172a26a8f0d0f0d170bd27e1c150ad204d8086a Mon Sep 17 00:00:00
>> > >> >> 2001
>> > >> >> From: =?UTF-8?q?Andreas=20F=C3=A4rber?= <afaerber at suse.de>
>> > >> >> Date: Sun, 27 Jul 2014 21:58:06 +0200
>> > >> >> Subject: [PATCH] ARM: dts: exynos5250: Add eDP/LVDS bridge to Spring
>> > >> >> MIME-Version: 1.0
>> > >> >> Content-Type: text/plain; charset=UTF-8
>> > >> >> Content-Transfer-Encoding: 8bit
>> > >> >>
>> > >> >> Signed-off-by: Ajay Kumar <ajaykumar.rs at samsung.com>
>> > >> >> [AF: Redone for v6]
>> > >> >> Signed-off-by: Andreas F??rber <afaerber at suse.de>
>> > >> >> ---
>> > >> >>
>> > >> >>  arch/arm/boot/dts/exynos5250-spring.dts | 32 +++++++++++++++++++++-
>> > >> >>  1 file changed, 31 insertions(+), 1 deletion(-)
>> > >> >>
>> > >> >> diff --git a/arch/arm/boot/dts/exynos5250-spring.dts
>> > >> >> b/arch/arm/boot/dts/exynos5250-spring.dts index
>> > >> >> 687dfab86bc8..517b1ff2bfdf 100644
>> > >> >> --- a/arch/arm/boot/dts/exynos5250-spring.dts
>> > >> >> +++ b/arch/arm/boot/dts/exynos5250-spring.dts
>> > >> >> @@ -64,10 +64,14 @@
>> > >> >>            vdd_pll-supply = <&s5m_ldo8_reg>;
>> > >> >>    };
>> > >> >>
>> > >> >> +  panel: panel {
>> > >> >> +          compatible = "simple-panel";
>> > >> >> +  };
>> > >> >
>> > >> > You can't do this. "simple-panel" isn't a valid panel model. It
>> > >> > should probably be removed from the platform_of_match table in the
>> > >> > driver.
>> > >>
>> > >> Okay, that means the Snow DT is wrong, too:
>> > >> https://patchwork.kernel.org/patch/4625441/
>> > >>
>> > >> And the others specify it as fallback:
>> > >> https://patchwork.kernel.org/patch/4625461/
>> > >> https://patchwork.kernel.org/patch/4625451/
>> > >
>> > > A quick grep shows that many (all?) devices that use DRM panels provide
>> > > simple-panel as fallback. That's probably fine as long as they also do
>> > > provide the specific model. But given that simple-panel does not have a
>> > > mode or physical size, I don't think even that makes sense.
>> >
>> > On snow, the bridge chip provides the display mode instead of the panel.
>> > That is why display was working for me.
>>
>> Okay, I suppose under some circumstances that might make sense. Although
>> it's still always the panel that dictates the display timings, so the
>> panel node needs to have a panel model specific compatible value with a
>> matching entry in the panel-simple driver so that it can even be used in
>> setups without a bridge.
>>
>> One other thing: how does the bridge know which mode to drive? I suspect
>> that it can drive more than one mode? Can it freely be configured or
>> does it have a predefined set of modes? If the latter, then according to
>> what you said above there needs to be a way to configure the bridge (via
>> DT?) so that it reports the mode matching the panel. I wonder if that
>> should be handled completely in code, so that for example a bridge has a
>> panel attached it can use the panel's .get_modes() and select a matching
>> mode among the set that it supports.
>
> Yes, pretty please :-) I don't think it would be a good idea to duplicate mode
> information in the bridge DT node, as that's not a property of the bridge.
> Querying the mode at runtime is in my opinion a much better option, and would
> also allow switching between different modes at runtime when that makes sense.
>
> Now, I'm not sure whether it should be the bridge driver querying the panel
> driver directly, or the display controller driver doing it and then
> configuring the bridge accordingly. The latter seems more generic to me and
> doesn't rely on the assumption that the bridge output will always be directly
> connected to a panel.
>
> --
> Regards,
>
> Laurent Pinchart
>