Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Kevin Hilman <khilman@baylibre.com>
To:     Neil Armstrong <narmstrong@baylibre.com>, a.hajda@samsung.com,
        Laurent.pinchart@ideasonboard.com
Cc:     jernej.skrabec@siol.net, Neil Armstrong <narmstrong@baylibre.com>,
        maxime.ripard@bootlin.com, jonas@kwiboo.se,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        hverkuil@xs4all.nl, linux-amlogic@lists.infradead.org
Subject: Re: [PATCH 5/5] drm/meson: Output in YUV444 if sink supports it
In-Reply-To: <20190520133753.23871-6-narmstrong@baylibre.com>
References: <20190520133753.23871-1-narmstrong@baylibre.com> <20190520133753.23871-6-narmstrong@baylibre.com>
Date:   Thu, 06 Jun 2019 10:16:20 -0700
Message-ID: <7hblzaeisb.fsf@baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Neil Armstrong <narmstrong@baylibre.com> writes:

> With the YUV420 handling, we can dynamically setup the HDMI output
> pixel format depending on the mode and connector info.
> So now, we can output in YUV444, which is the native video pipeline
> format, directly to the HDMI Sink if it's supported without
> necessarily involving the HDMI Controller CSC.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_dw_hdmi.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 5d67e2beba58..8bf9db7f39a4 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -723,12 +723,23 @@ static int meson_venc_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
>  	struct drm_display_mode *mode = &crtc_state->mode;
>  	bool is_hdmi2_sink =
>  		conn_state->connector->display_info.hdmi.scdc.supported;
> +	bool specify_out_format = false;
> +	u32 out_format;
>  
>  	if (drm_mode_is_420_only(info, mode) ||
>  	    (!is_hdmi2_sink && drm_mode_is_420_also(info, mode)))
>  		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_UYYVYY8_0_5X24;
> -	else
> +	else {

nit: if the else has {} you should add to the 'if' (even if the if side
is a single statement): c.f. end of this section of CodingStyle:
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces

>  		dw_hdmi->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
> +		if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444) {
> +			out_format = MEDIA_BUS_FMT_YUV8_1X24;
> +			specify_out_format = true;
> +		}
> +	}
> +
> +	/* Set a connector bus format if required */
> +	drm_display_info_set_bus_formats(info, &out_format,
> +					 (specify_out_format ? 1 : 0));
>  

Otherwise,

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

Kevin