Hi Niklas,
Thank you for the patch.
On Sat, Feb 11, 2023 at 05:57:14PM +0100, Niklas Söderlund wrote:
> Most Gen3 R-Car devices have HDMI as the default video input source,
> align Draak with them and make HDMI the default. While at it move the
> bus properties to the VIN node where they can be consumed correctly by
> the driver.
I'm fine with the idea, but I'm wondering if this matches the default
DIP switches configuration that boards are shipped with. This being
said, when I check the switches on my board to test HDMI input a few
days ago, I realized they were set to a hybrid configuration, so maybe
we should just roll our eyes and merge this :-)
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/renesas/draak.dtsi | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/draak.dtsi b/arch/arm64/boot/dts/renesas/draak.dtsi
> index ef3bb835d5c0..e248866c7871 100644
> --- a/arch/arm64/boot/dts/renesas/draak.dtsi
> +++ b/arch/arm64/boot/dts/renesas/draak.dtsi
> @@ -356,11 +356,10 @@ port@3 {
> * CVBS and HDMI inputs through SW[49-53]
> * switches.
> *
> - * CVBS is the default selection, link it to
> - * VIN4 here.
> + * HDMI is the default selection, leave CVBS
> + * not connected here.
> */
> adv7180_out: endpoint {
> - remote-endpoint = <&vin4_in>;
> };
Endpoints are required to have a remote-endpoint property, so you should
drop the endpoint completely. This will require a change in patch 2/2 as
you'll have to create the endpoint there.
> };
> };
> @@ -423,13 +422,11 @@ port@2 {
> * CVBS and HDMI inputs through SW[49-53]
> * switches.
> *
> - * CVBS is the default selection, leave HDMI
> - * not connected here.
> + * HDMI is the default selection, link it to
> + * VIN4 here.
> */
> adv7612_out: endpoint {
> - pclk-sample = <0>;
> - hsync-active = <0>;
> - vsync-active = <0>;
This will cause the bus type to change from parallel to BT656. Is that
desired ? If not, I'd set the bus-type property explicitly. Actually,
I'd set it explicitly in any case.
This change is worth being split to a separate patch.
> + remote-endpoint = <&vin4_in>;
> };
> };
> };
> @@ -580,8 +577,8 @@ usb0_pins: usb0 {
> function = "usb0";
> };
>
> - vin4_pins_cvbs: vin4 {
> - groups = "vin4_data8", "vin4_sync", "vin4_clk";
> + vin4_pins: vin4 {
> + groups = "vin4_data24", "vin4_sync", "vin4_clk";
> function = "vin4";
> };
> };
> @@ -729,7 +726,7 @@ &usb2_phy0 {
> };
>
> &vin4 {
> - pinctrl-0 = <&vin4_pins_cvbs>;
> + pinctrl-0 = <&vin4_pins>;
> pinctrl-names = "default";
>
> status = "okay";
> @@ -737,7 +734,10 @@ &vin4 {
> ports {
> port {
> vin4_in: endpoint {
> - remote-endpoint = <&adv7180_out>;
> + pclk-sample = <0>;
> + hsync-active = <0>;
> + vsync-active = <0>;
> + remote-endpoint = <&adv7612_out>;
> };
> };
> };
--
Regards,
Laurent Pinchart
