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