> -----Original Message----- > From: linux-samsung-soc-ow...@vger.kernel.org [mailto:linux-samsung-soc- > ow...@vger.kernel.org] On Behalf Of Rahul Sharma > Sent: Thursday, September 05, 2013 3:04 PM > To: Inki Dae > Cc: Sean Paul; Rahul Sharma; linux-samsung-soc; dri-devel; kgene.kim; > sw0312.kim; Lucas Stach; Tomasz Figa; Sylwester Nawrocki; sunil joshi; > Shirish S > Subject: Re: [PATCH 1/7] drm/exynos: move hdmiphy related code to hdmiphy > driver > > On 5 September 2013 10:52, Inki Dae <inki....@samsung.com> wrote: > >> >> >> >> +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, > >> >> >> >> + }, > >> >> >> >> + }, > >> >> >> >> + { > >> >> >> >> + .pixel_clock = 27027000, > >> >> >> >> + .conf = { > >> >> >> >> + 0x01, 0x05, 0x00, 0xD4, 0x10, 0x9C, 0x09, > >> > 0x64, > >> >> >> >> + 0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, > >> > 0x87, > >> >> >> >> + 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, > >> > 0xE0, > >> >> >> >> + 0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, > >> > 0x00, > >> >> >> >> + }, > >> >> >> >> + }, > >> >> >> >> + { > >> >> >> >> + .pixel_clock = 74176000, > >> >> >> >> + .conf = { > >> >> >> >> + 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xef, > >> > 0x5B, > >> >> >> >> + 0x6D, 0x10, 0x01, 0x51, 0xef, 0xF3, 0x54, > >> > 0xb9, > >> >> >> >> + 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, > >> > 0xE0, > >> >> >> >> + 0x22, 0x40, 0xa5, 0x26, 0x01, 0x00, 0x00, > >> > 0x00, > >> >> >> >> + }, > >> >> >> >> + }, > >> >> >> >> + { > >> >> >> >> + .pixel_clock = 74250000, > >> >> >> >> + .conf = { > >> >> >> >> + 0x01, 0x05, 0x00, 0xd8, 0x10, 0x9c, 0xf8, > >> > 0x40, > >> >> >> >> + 0x6a, 0x10, 0x01, 0x51, 0xff, 0xf1, 0x54, > >> > 0xba, > >> >> >> >> + 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, > >> > 0xe0, > >> >> >> >> + 0x22, 0x40, 0xa4, 0x26, 0x01, 0x00, 0x00, > >> > 0x00, > >> >> >> >> + }, > >> >> >> >> + }, > >> >> >> >> + { > >> >> >> >> + .pixel_clock = 148500000, > >> >> >> >> + .conf = { > >> >> >> >> + 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xf8, > >> > 0x40, > >> >> >> >> + 0x6A, 0x18, 0x00, 0x51, 0xff, 0xF1, 0x54, > >> > 0xba, > >> >> >> >> + 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, > >> > 0xE0, > >> >> >> >> + 0x22, 0x40, 0xa4, 0x26, 0x02, 0x00, 0x00, > >> > 0x00, > >> >> >> >> + }, > >> >> >> >> + }, > >> >> >> >> +}; > >> >> >> >> + > >> >> >> > > >> >> >> > 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? > >> >> >> > > >> >> > > >> >> > Are these really board-specific values? > >> >> > >> >> According to your hardware people: > >> >> > >> >> "If the signal pattern doesn't change for new board, the phy setting > >> >> is same as the current board. But if changed, you need to confirm > with > >> >> measurement of signals, because it may vary slightly by resistance > of > >> >> board pattern" > >> >> > >> > > >> > Right. it seems that the phy configuration should be adjusted > according > >> to > >> > PCB environment: OSC clock rate, 24MHz or 27MHz, could be decided by > PCB > >> > even though most PCBs use 27MHz. > >> > > >> >> That indicates to me that we might need to tweak these on a per- > board > >> >> basis. > >> >> > >> >> In the 5420 datasheet, there are a few register descriptions > available > >> >> for the phy. 0x145D0004 is CLK_SEL which seems like it would be > >> >> board-specific, same with TX_* registers. > >> >> > >> > > >> > And we can select HDMI Tx PHY internal PLL input clock by setting > >> CLK_SEL. > >> > Ok, Shirish's patch set is reasonable to me. However, that patch set > >> should > >> > be rebased on top of Rahul's patch set. Shirish and Rahul, please re- > >> post > >> > your patch set after discussing how to rebase these patch set. > >> > > >> > Thanks, > >> > Inki Dae > >> > > >> > >> In that case, we need to test the phy confs for all the exynos boards, > >> supported in > >> mainline. Probably needs a analyser as well to precisely compare the > >> deviation. > > > > Shirish patch adds phy config data only to arndale and smdk5250 boards, > and > > these config data should have each board specific values. Therefore, for > > other boards, shouldn't correct phy config data suitable to their boards > be > > added to their board dts files? Is the above analyzer really needed? > > > > Sorry, I had only seen his patches for chromium tree. In mainline > version, he added for 5250 as well. But both sets (for arndale and > smdk) are exactly same as original 5250 configs which also works > with 4412 origen. > > Some problem has been identified during conformance testing for > 5420 peach board, which happens with analyser. It was always > working fine on the TV sets that I have. @Shirish/Sean please > correct me if wrong. > > >> Shirish patch is only for 5420 Peach board. Else, to start with we can > >> mark > >> phyconf as optional property which overrides the default Phy Confs for > >> given SoC. > > > > Hm.... you mean that hdmiphy driver use the default phy config data in > > driver; most boards use the same data, and only in special case; a board > > uses different OSC clock rate, the hdmiphy driver use phy config data > from > > dts file checking hdmiphy-confs property? > > > > Yes. I meant same. I don't see the real need to duplicate so much > of data in all board dts files. We can add it for a particular board, if > really required. >
Yes, reasonable to me. It's not good that board dts files have same phy config data. How about using the phy config data from dts file if hdmiphy-confs property exists, otherwise using default phy config data then? This means that we don't need to remove the phy config data from driver; that will be used as default values. Rahul, let's go if there is other opinion. we SHOULD MERGE these this time.:) Thanks, Inki Dae > Regards, > Rahul Sharma. > > > > >> > >> regards, > >> Rahul Sharma. > >> > >> >> Sean > >> >> > >> >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung- > soc" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
