Re: [PATCH 1/7] drm/exynos: move hdmiphy related code to hdmiphy driver

From: Shirish S <shirish@chromium.org>
To: Inki Dae <inki.dae@samsung.com>
Cc: Mark Rutland <Mark.Rutland@arm.com>,
	devicetree@vger.kernel.org,
	linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	Pawel Moll <Pawel.Moll@arm.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	"sw0312.kim" <sw0312.kim@samsung.com>,
	sunil joshi <joshi@samsung.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"kgene.kim" <kgene.kim@samsung.com>,
	Rob Herring <rob.herring@calxeda.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	Kumar Gala <galak@codeaurora.org>,
	Sylwester Nawrocki <sylvester.nawrocki@gmail.com>,
	Rahul Sharma <rahul.sharma@samsung.com>
Subject: Re: [PATCH 1/7] drm/exynos: move hdmiphy related code to hdmiphy driver
Date: Mon, 28 Oct 2013 11:36:30 +0530	[thread overview]
Message-ID: <CAHvYUDDeSr9GBY8R7Hp6F9z9A4QP=TtVa_+whv3F8j4STJtbJA@mail.gmail.com> (raw)
In-Reply-To: <CAHvYUDC1GprC1xQcCSdRvSafe_yyihDsrzrTvfS-Ey-hELOgjA@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 5045 bytes --]

Hi,
I have uploaded patch set 2, with only required registers being moved to dt
file.
Thanks,
Shirish S

On Thu, Oct 3, 2013 at 7:58 AM, Shirish S <shirish@chromium.org> wrote:

> Hi,
> First of all sorry for the late response,
>
>
> On Tue, Oct 1, 2013 at 10:09 AM, Inki Dae <inki.dae@samsung.com> wrote:
>
>>
>>
>> > -----Original Message-----
>> > From: Sylwester Nawrocki [mailto:sylvester.nawrocki@gmail.com]
>> > Sent: Monday, September 30, 2013 7:09 AM
>> > To: Inki Dae
>> > Cc: Rahul Sharma; devicetree@vger.kernel.org; linux-samsung-soc;
>> > sw0312.kim; sunil joshi; dri-devel; kgene.kim; Shirish S; Sylwester
>> > Nawrocki; Rahul Sharma; Stephen Warren; Mark Rutland; Kumar Gala; Pawel
>> > Moll; Rob Herring; Sean Paul
>> > Subject: Re: [PATCH 1/7] drm/exynos: move hdmiphy related code to
>> hdmiphy
>> > driver
>> >
>> > On 09/28/2013 06:10 PM, Inki Dae wrote:
>> > >> Any opinion from Device-Tree folks?
>> > >>
>> > >> IMO, we should have same consensus on Shirish patches before
>> proceeding.
>> > >
>> > > Rahul, it seems that DT people have no interest in this issue. So
>> let's
>> > > have a consensus about this issue internally.
>> > >
>> > > To Mr. Kyungmin, Sylwester, Kukjin Kim, and Tomasz,
>> > > How about keeping hdmiphy config data in each board dts file?
>> >
>> > Please don't use HTML and quote only relevant part of e-mails. Otherwise
>> > there are good chances your messages end up in people's spam box.
>> >
>>
>> Ah, I missed checking text mode. Sorry about this. :)
>>
>>
>>
>> > It often helps to Cc a DT binding maintainer directly.
>> >
>>
>> Good idea.
>>
>> > Then, you consider moving the HDMI phy configuration to the device tree.
>> > As Sean suggested in this thread:
>> >
>>
>> Right.
>>
>> > ">> +static struct hdmiphy_config hdmiphy_4210_configs[] = {
>> > >> +       {
>> > >> +               .pixel_clock = 27000000,
>> > >> +               .conf = {
>> > >> +                       0x01, 0x05, 0x00, 0xD8, 0x10, 0x1C, 0x30,
>> 0x40,
>> > >> +                       0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54,
>> 0x87,
>> > >> +                       0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10,
>> 0xE0,
>> > >> +                       0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00,
>> 0x00,
>> > >> +               },
>> > >> +       },
>> > [trimmed couple more entries]
>> > >> +};
>> > >>
>> > > Are you aware of the effort to move these to dt? Since these are
>> > > board-specific values, it seems incorrect to apply them universally.
>> > > Shirish has uploaded a patch to the chromium review site to push these
>> > > into dt (https://chromium-review.googlesource.com/#/c/65581). Maybe
>> > > you can work that into your patch set?"
>> >
>> > The configuration data is 64 bytes of the register values IIUC. Would it
>> > be
>> > possible to figure out exact meaning of each byte ? Do all of these
>> bytes
>>
>> Right, but the user manual doesn't describe that enough so we might need
>> to
>> inquire for what it means to design team.
>>
>> > need to be changed per board ? Perhaps we can have per SoC static tables
>> > in
>> > the PHY driver and be overwriting only some of the bytes with values
>> read
>> > from device tree ?
>> >
>> > AFAIR firmware-like binary blobs (register address/value pairs) are not
>> > supposed to be stored in DT.
>> >
>> > If there is no detailed documentation for theese PHY configuration
>> tables
>> > I guess there is no choice but to put these raw 64 bytes in DT.
>> Presumably
>> > this should be a an required property defined only by board dts, since
>> > those
>> > values are PCB design dependent.
>> >
>> > However, if not all boards need tweaking this configuration data, then
>> it
>> > could make sense to define recommended per-SoC tables in the driver and
>> > overwrite from DT only if it is specified there for a specific board.
>> > If we can come up with universal configuration for a SoC and selected
>> > pixel
>> > clock frequency then it could likely be better to store that in the
>> driver
>> > rather than in DT.
>> >
>>
>> Thanks you your opinion. However, we need to make sure how we take care of
>> the PHY configuration values. So I will have two steps to merge this pages
>> set.
>>
>> To Rahul,
>> Could you post only your patch set regardless of Shirish's patch? I will
>> merge your patch set first because as is, Exynos drm hdmi driver is
>> broken.
>> And, we need more discussions about Shirish patch. So I will not merge
>> this
>> patch until we have a consensus about it.
>>
>> To Shirish,
>> For your patch, it seems that you need to make sure to figure out exact
>> meaning of each byte of the PHY configuration values first. Maybe you need
>> to inquire for that to hardware or design team. And please separate the
>> values into common and specific parts if needed.
>>
>> Agreed, I shall request our hardware team to provide description about
> the phy values, and will update the patch, once i receive the same.
>
>
>>
>> Thanks,
>> Inki Dae
>>
>> > --
>> > Thanks,
>> > Sylwester
>>
>> Thanks,
> Shirish S
>

[-- Attachment #1.2: Type: text/html, Size: 6813 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel