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

[Rahul Sharma]

On 5 September 2013 10:52, Inki Dae <inki.dae at 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.

Regards,
Rahul Sharma.

>
>>
>> regards,
>> Rahul Sharma.
>>
>> >> Sean
>> >>
>> >>
>