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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6ACEC4167B for ; Tue, 7 Nov 2023 15:42:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344041AbjKGPm0 (ORCPT ); Tue, 7 Nov 2023 10:42:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1343863AbjKGPmX (ORCPT ); Tue, 7 Nov 2023 10:42:23 -0500 X-Greylist: delayed 97395 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Tue, 07 Nov 2023 07:42:21 PST Received: from mailrelay1-1.pub.mailoutpod2-cph3.one.com (mailrelay1-1.pub.mailoutpod2-cph3.one.com [46.30.211.176]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7CE8A94 for ; Tue, 7 Nov 2023 07:42:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ravnborg.org; s=rsa2; h=in-reply-to:content-type:mime-version:references:message-id:subject:cc:to: from:date:from; bh=r4pS3nzg1xHsTPROp8NMUMZcT6aPPgnVU+KRkpioBHA=; b=gjsapIyc2vwjxnaisvqK9yQebWHeP7F/rxPVU/9qvnVqohA3dIo2HPJ72ldLQQ1nUS8gCOv4jzuSo B9+7A5BI3UYe23FGkZbn4NqMTpPLQb0t94Z4K5zgYYipdhkJARHTWiHivdV2TZWOXmU54JiMNdwpCX QsaO39o1dExXbjsB5LlxPYlFU5rjD9JOR2kTDVw1zi42hXYQWEbJIcmkYi/+BKY9ahfFowZ/OvBlZG EyYVFsbLxlZLeZB2HLJBilE2grCc7mgwPBhHVx9ScabZ6ZbP+rAEmDZ8lvQt7TiY+AMYDhbjodtOxt HYyFP7cbjbZiVQjXFZl08ITh+VCMjnQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ravnborg.org; s=ed2; h=in-reply-to:content-type:mime-version:references:message-id:subject:cc:to: from:date:from; bh=r4pS3nzg1xHsTPROp8NMUMZcT6aPPgnVU+KRkpioBHA=; b=AqDvLY68qSvpYK0M6v2AFgdtAJSxj+atFt3Pbp78PJGiSuGbTqa+15txBENi21GWvuCMEC4LDLWTu SEss8K9BQ== X-HalOne-ID: 15d52a21-7d84-11ee-8d75-2b733b0ff8f0 Received: from ravnborg.org (2-105-2-98-cable.dk.customer.tdc.net [2.105.2.98]) by mailrelay1 (Halon) with ESMTPSA id 15d52a21-7d84-11ee-8d75-2b733b0ff8f0; Tue, 07 Nov 2023 15:41:16 +0000 (UTC) Date: Tue, 7 Nov 2023 16:41:15 +0100 From: Sam Ravnborg To: Aradhya Bhatia Cc: Nishanth Menon , Vignesh Raghavendra , Devarsh Thakkar , Jan Kiszka , DRI Development List , Laurent Pinchart , Andrzej Hajda , Robert Foss , Francesco Dolcini , Jernej Skrabec , Thomas Zimmermann , Jonas Karlman , Maxime Ripard , Neil Armstrong , Jayesh Choudhary , Tomi Valkeinen , Linux Kernel List , Boris Brezillon , Jyri Sarha Subject: Re: [PATCH] drm/bridge: tc358767: Support input format negotiation hook Message-ID: <20231107154115.GA100782@ravnborg.org> References: <20231030192846.27934-1-a-bhatia1@ti.com> <20231106123800.GC47195@ravnborg.org> <7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Aradhya, On Tue, Nov 07, 2023 at 01:17:03AM +0530, Aradhya Bhatia wrote: > Hi Sam, > > Thank you for the suggestion! > > On 06-Nov-23 18:08, Sam Ravnborg wrote: > > Hi Aradhya, > > > > On Tue, Oct 31, 2023 at 12:58:46AM +0530, Aradhya Bhatia wrote: > >> With new connector model, tc358767 will not create the connector, when > >> DRM_BRIDGE_ATTACH_NO_CONNECTOR is set and display-controller driver will > >> rely on format negotiation to setup the encoder format. > >> > >> Add the missing input-format negotiation hook in the > >> drm_bridge_funcs to complete DRM_BRIDGE_ATTACH_NO_CONNECTOR support. > >> > >> Input format is selected to MEDIA_BUS_FMT_RGB888_1X24 as default, as is > >> the case with older model. > >> > >> Reported-by: Jan Kiszka > >> Signed-off-by: Aradhya Bhatia > >> --- > >> > >> Notes: > >> > >> * Since I do not have hardware with me, this was just build tested. I would > >> appreciate it if someone could test and review it, especically somebody, who > >> uses the bridge for DPI/DSI to eDP format conversion. > >> > >> * The Toshiba TC358767 bridge is not enabled in arm64 defconfig by default, > >> when it should be. Hence, I sent a quick patch[0] earlier. > >> > >> [0]: https://lore.kernel.org/all/20231030152834.18450-1-a-bhatia1@ti.com/ > >> > >> drivers/gpu/drm/bridge/tc358767.c | 25 +++++++++++++++++++++++++ > >> 1 file changed, 25 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > >> index ef2e373606ba..0affcefdeb1c 100644 > >> --- a/drivers/gpu/drm/bridge/tc358767.c > >> +++ b/drivers/gpu/drm/bridge/tc358767.c > >> @@ -1751,6 +1751,30 @@ tc_dpi_atomic_get_input_bus_fmts(struct drm_bridge *bridge, > >> return input_fmts; > >> } > >> > >> +static u32 * > >> +tc_edp_atomic_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) > >> +{ > >> + u32 *input_fmts; > >> + > >> + *num_input_fmts = 0; > >> + > >> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), > >> + GFP_KERNEL); > >> + if (!input_fmts) > >> + return NULL; > >> + > >> + /* This is the DSI/DPI-end bus format */ > >> + input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24; > >> + *num_input_fmts = 1; > >> + > >> + return input_fmts; > >> +} > > > > You could benefit from using the helper: > > drm_atomic_helper_bridge_propagate_bus_fmt() > > You are right! > > Upon taking a second look, I realize that the bridge chain works with > MEDIA_BUS_FMT_FIXED bus format, when tc358767 is being used in DPI/DSI > to eDP mode (because the panel-bridge does not have a get_output_bus_fmt > hook, and uses the same helper for its get_input_bus_fmt hook). My patch > creates a deviation from that, by forcing MEDIA_BUS_FMT_RGB888_1X24 even > when eDP is involved. > > Using the helper here, will certainly address this deviation. > > However, for the DPI/DSI to DP mode, MEDIA_BUS_FMT_RGB888_1X24 bus > format is required, and *just* using the helper as its get_input_bus_fmt > hook, might not be enough. > > Since tc358767 is the last bridge in DPI/DSI to DP mode, the > output_fmt parameter get defaulted to MEDIA_BUS_FMT_FIXED too, as there > is no get_output_bus_fmt hook present in the driver. If we simply us > the helper here, the input_fmt will also get set to MEDIA_BUS_FMT_FIXED. > This too is an unwanted deviation. > > It seems like the right way to address both the cases, would be by > adding the get_output_bus_fmt hook that sets output_fmt to > MEDIA_BUS_FMT_RGB888_1X24, as well as using the helper as the > get_input_bus_fmt hook. > > If this seems good to you too, I will send a new version of Tomi's > series[0] which incorporates this patch. I never managed to fully wrap my head around the bus fmt negotiation, and as I am trying to recover from a flu this is not the time to try. Your explanations sounds like you have grasped it so I suggest to move ahead. Sam