From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4B3B9C54E58 for ; Tue, 19 Mar 2024 00:05:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=H7/SWTaFDXZ7wiGujOA4I42R7/aFLrxDQlX/vJmfkE4=; b=zqf8DEVX8e2Hd0 H2xgxembuYbAa3a5EZFxevPRzyWRxK1QZ/9LpvpMyBkxkUe0YV0ezrkKz7lVjXpu8z3XW2GrRG9ZD 7qPquwBKhRuArJsISUc/tgOY5IPS0tKwBRrwj1PGd5YKHjEIiX+oVaPjzEN2ItQ3b9gJd42y2AiI0 zjvZDGRU8zIQFyHBLE5nayDLQCjZcHQ3PG/em4/Ayp6RZF2N82H60xWOJMu04EmmMH/AyYWv3Ao6s eUghOCaBZdN6RhVvLCX5E/VRTsZ6FAaj7mJP2j16fcOrT2SvkMpZpdcv/OucXvRmt+7CI+JJCY0HP g3SxwNva9Ygb/Yk19ffA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmMyd-0000000AdPt-1nfV; Tue, 19 Mar 2024 00:05:11 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmMyY-0000000AdOi-1F09 for linux-arm-kernel@lists.infradead.org; Tue, 19 Mar 2024 00:05:08 +0000 Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 730F0480; Tue, 19 Mar 2024 01:04:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710806676; bh=qUH+g0HruDEXB3YbYdk5MPO634/Ais9TzewOfl8eAFI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Z3zl3nbpBuIjWC7upKjiBFujOL3nIfU43/iXfgDl8B/qFWxahSarqHuPFyqKEvG5y 8p2wAMa7C0j3sPZ6wl0qG8Ne3A4cnlYd7wIQrpEsCH74lEUx+Qz1/m2mwhKtlhizS4 Ita7Df/heP/ubrS3UpTcviglMV/F8eAWR+pZL6KE= Date: Tue, 19 Mar 2024 02:05:00 +0200 From: Laurent Pinchart To: Anatoliy Klymenko Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Michal Simek , Andrzej Hajda , Neil Armstrong , Robert Foss , Jonas Karlman , Jernej Skrabec , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Mauro Carvalho Chehab , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-media@vger.kernel.org Subject: Re: [PATCH v2 3/8] drm: xlnx: zynqmp_dpsub: Anounce supported input formats Message-ID: <20240319000500.GU13682@pendragon.ideasonboard.com> References: <20240312-dp-live-fmt-v2-0-a9c35dc5c50d@amd.com> <20240312-dp-live-fmt-v2-3-a9c35dc5c50d@amd.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240312-dp-live-fmt-v2-3-a9c35dc5c50d@amd.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240318_170506_534139_79957C31 X-CRM114-Status: GOOD ( 33.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Anatoliy, Thank you for the patch. On Tue, Mar 12, 2024 at 05:55:00PM -0700, Anatoliy Klymenko wrote: > DPSUB in bridge mode supports multiple input media bus formats. > > Announce the list of supported input media bus formats via > drm_bridge.atomic_get_input_bus_fmts callback. > Introduce a set of live input formats, supported by DPSUB. > Rename zynqmp_disp_layer_drm_formats() to zynqmp_disp_layer_formats() to > reflect semantics for both live and non-live layer format lists. > > Signed-off-by: Anatoliy Klymenko > --- > drivers/gpu/drm/xlnx/zynqmp_disp.c | 67 +++++++++++++++++++++++++++++++++----- > drivers/gpu/drm/xlnx/zynqmp_disp.h | 4 +-- > drivers/gpu/drm/xlnx/zynqmp_dp.c | 26 +++++++++++++++ > drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +- > 4 files changed, 88 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c > index e6d26ef60e89..af851190f447 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -77,12 +78,16 @@ enum zynqmp_dpsub_layer_mode { > /** > * struct zynqmp_disp_format - Display subsystem format information > * @drm_fmt: DRM format (4CC) > + * @bus_fmt: Media bus format > * @buf_fmt: AV buffer format > * @swap: Flag to swap R & B for RGB formats, and U & V for YUV formats > * @sf: Scaling factors for color components > */ > struct zynqmp_disp_format { > - u32 drm_fmt; > + union { > + u32 drm_fmt; > + u32 bus_fmt; > + }; I'm not a big fan of the union, but I can live with it. > u32 buf_fmt; > bool swap; > const u32 *sf; > @@ -182,6 +187,12 @@ static const u32 scaling_factors_565[] = { > ZYNQMP_DISP_AV_BUF_5BIT_SF, > }; > > +static const u32 scaling_factors_666[] = { > + ZYNQMP_DISP_AV_BUF_6BIT_SF, > + ZYNQMP_DISP_AV_BUF_6BIT_SF, > + ZYNQMP_DISP_AV_BUF_6BIT_SF, > +}; > + > static const u32 scaling_factors_888[] = { > ZYNQMP_DISP_AV_BUF_8BIT_SF, > ZYNQMP_DISP_AV_BUF_8BIT_SF, > @@ -364,6 +375,36 @@ static const struct zynqmp_disp_format avbuf_gfx_fmts[] = { > }, > }; > > +/* List of live video layer formats */ > +static const struct zynqmp_disp_format avbuf_live_fmts[] = { > + { > + .bus_fmt = MEDIA_BUS_FMT_RGB666_1X18, > + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_6 | > + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB, > + .sf = scaling_factors_666, > + }, { > + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X24, > + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 | > + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_RGB, > + .sf = scaling_factors_888, > + }, { > + .bus_fmt = MEDIA_BUS_FMT_UYVY8_1X16, > + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 | > + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422, > + .sf = scaling_factors_888, > + }, { > + .bus_fmt = MEDIA_BUS_FMT_VUY8_1X24, > + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_8 | > + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV444, > + .sf = scaling_factors_888, > + }, { > + .bus_fmt = MEDIA_BUS_FMT_UYVY10_1X20, > + .buf_fmt = ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_BPC_10 | > + ZYNQMP_DISP_AV_BUF_LIVE_CONFIG_FMT_YUV422, > + .sf = scaling_factors_101010, > + }, > +}; > + > static u32 zynqmp_disp_avbuf_read(struct zynqmp_disp *disp, int reg) > { > return readl(disp->avbuf.base + reg); > @@ -883,16 +924,17 @@ zynqmp_disp_layer_find_format(struct zynqmp_disp_layer *layer, > } > > /** > - * zynqmp_disp_layer_drm_formats - Return the DRM formats supported by the layer > + * zynqmp_disp_layer_formats - Return DRM or media bus formats supported by > + * the layer > * @layer: The layer > * @num_formats: Pointer to the returned number of formats > * > - * Return: A newly allocated u32 array that stores all the DRM formats > + * Return: A newly allocated u32 array that stores all DRM or media bus formats > * supported by the layer. The number of formats in the array is returned > * through the num_formats argument. > */ > -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer, > - unsigned int *num_formats) > +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer, > + unsigned int *num_formats) > { > unsigned int i; > u32 *formats; > @@ -1131,6 +1173,11 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp) > .num_channels = 1, > }, > }; > + static const struct zynqmp_disp_layer_info live_layer_info = { > + .formats = avbuf_live_fmts, > + .num_formats = ARRAY_SIZE(avbuf_live_fmts), > + .num_channels = 0, > + }; > > unsigned int i; > int ret; > @@ -1140,12 +1187,16 @@ static int zynqmp_disp_create_layers(struct zynqmp_disp *disp) > > layer->id = i; > layer->disp = disp; > - layer->info = &layer_info[i]; > /* For now assume dpsub works in either live or non-live mode for both layers. While are it, could you please turn this into /* * For now assume dpsub works in either live or non-live mode for both layers. with a blank line just above it ? > * Hybrid mode is not supported yet. > */ > - layer->mode = disp->dpsub->dma_enabled ? ZYNQMP_DPSUB_LAYER_NONLIVE > - : ZYNQMP_DPSUB_LAYER_LIVE; > + if (disp->dpsub->dma_enabled) { > + layer->mode = ZYNQMP_DPSUB_LAYER_NONLIVE; > + layer->info = &layer_info[i]; > + } else { > + layer->mode = ZYNQMP_DPSUB_LAYER_LIVE; > + layer->info = &live_layer_info; > + } > > ret = zynqmp_disp_layer_request_dma(disp, layer); > if (ret) > diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.h b/drivers/gpu/drm/xlnx/zynqmp_disp.h > index 9b8b202224d9..88c285a12e23 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_disp.h > +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.h > @@ -50,8 +50,8 @@ int zynqmp_disp_setup_clock(struct zynqmp_disp *disp, > void zynqmp_disp_blend_set_global_alpha(struct zynqmp_disp *disp, > bool enable, u32 alpha); > > -u32 *zynqmp_disp_layer_drm_formats(struct zynqmp_disp_layer *layer, > - unsigned int *num_formats); > +u32 *zynqmp_disp_layer_formats(struct zynqmp_disp_layer *layer, > + unsigned int *num_formats); > void zynqmp_disp_layer_enable(struct zynqmp_disp_layer *layer); > void zynqmp_disp_layer_disable(struct zynqmp_disp_layer *layer); > void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer, > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c > index 04b6bcac3b07..a0d169ac48c0 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > @@ -1568,6 +1568,31 @@ static const struct drm_edid *zynqmp_dp_bridge_edid_read(struct drm_bridge *brid > return drm_edid_read_ddc(connector, &dp->aux.ddc); > } > > +static u32 * > +zynqmp_dp_bridge_get_input_bus_fmts(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state, > + u32 output_fmt, > + unsigned int *num_input_fmts) > +{ > + struct zynqmp_dp *dp = bridge_to_dp(bridge); > + struct zynqmp_disp_layer *layer; > + enum zynqmp_dpsub_layer_id layer_id; > + > + if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_VIDEO)) > + layer_id = ZYNQMP_DPSUB_LAYER_VID; > + else if (dp->dpsub->connected_ports & BIT(ZYNQMP_DPSUB_PORT_LIVE_GFX)) > + layer_id = ZYNQMP_DPSUB_LAYER_GFX; > + else { > + *num_input_fmts = 0; > + return NULL; > + } You need curly braces around all branches if one of them has multiple statements. Given that the above pattern is repeated twice already, a helper function that returns the layer pointer would be useful. Then you could simply write layer = ...(dp); if (!layer) { *num_input_fmts = 0; return NULL; } With these small issues addressed, Reviewed-by: Laurent Pinchart > + layer = dp->dpsub->layers[layer_id]; > + > + return zynqmp_disp_layer_formats(layer, num_input_fmts); > +} > + > static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = { > .attach = zynqmp_dp_bridge_attach, > .detach = zynqmp_dp_bridge_detach, > @@ -1580,6 +1605,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = { > .atomic_check = zynqmp_dp_bridge_atomic_check, > .detect = zynqmp_dp_bridge_detect, > .edid_read = zynqmp_dp_bridge_edid_read, > + .atomic_get_input_bus_fmts = zynqmp_dp_bridge_get_input_bus_fmts, > }; > > /* ----------------------------------------------------------------------------- > diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c > index 43bf416b33d5..bf9fba01df0e 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c > @@ -152,7 +152,7 @@ static int zynqmp_dpsub_create_planes(struct zynqmp_dpsub *dpsub) > unsigned int num_formats; > u32 *formats; > > - formats = zynqmp_disp_layer_drm_formats(layer, &num_formats); > + formats = zynqmp_disp_layer_formats(layer, &num_formats); > if (!formats) > return -ENOMEM; > > -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel