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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 78143105A591 for ; Thu, 12 Mar 2026 13:18:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C6CD610E9E4; Thu, 12 Mar 2026 13:18:46 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=collabora.com header.i=@collabora.com header.b="bvdCyCj6"; dkim-atps=neutral Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) by gabe.freedesktop.org (Postfix) with ESMTPS id 564A910E9E4; Thu, 12 Mar 2026 13:18:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1773321525; bh=GE9L1zaNRQfi+E3nkLw0E2ueYNvEqDq6XZiDAGuAmpY=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=bvdCyCj6KKkeGK8qpgUgRZ3jMv2Lb5CfRP/77c2hJnBkAX/MyJm0HpDVj8ze26Bd3 K02ejEaWiSeMhOZk3qTH1R/xN8kswoxY9YdH/T47ynv09VLpB5Pcz6p+mDJiOnsD/v dV1lQoomGAtsel3AqP4vOsqzHBwNQUGhaepqTIzZ2hQyrw2ygAGwcFl9ir4PDCWwU6 B827F91px2hffOOQwIyZs+koxV7AIDhNDjDsrYnnBpm5bzhNrEKFN0hpe7iNo4cqeF Qq/AXJZLetreeM0majZRztu4yD7/v6MLSE0431t30j+Fn2ulKWUyJOsMKEzhZuXDaq 5Rp87mOUGNx7A== Received: from [192.168.1.100] (2-237-20-237.ip236.fastwebnet.it [2.237.20.237]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by bali.collaboradmins.com (Postfix) with ESMTPSA id 6BE7517E026C; Thu, 12 Mar 2026 14:18:44 +0100 (CET) Message-ID: Date: Thu, 12 Mar 2026 14:18:43 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] drm/msm/dsi: Restore widebus calculation for CMDMode panels To: Marijn Suijten , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , David Airlie , Simona Vetter , Pengyu Luo Cc: ~postmarketos/upstreaming@lists.sr.ht, Konrad Dybcio , Martin Botka , Jami Kettunen , Dmitry Baryshkov , linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org> From: AngeloGioacchino Del Regno Content-Language: en-US In-Reply-To: <20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Il 11/03/26 23:31, Marijn Suijten ha scritto: > Commit ac47870fd795 ("drm/msm/dsi: fix hdisplay calculation when > programming dsi registers") makes a false and ungrounded statement that > "Since CMD mode does not use this, we can remove !(msm_host->mode_flags > & MIPI_DSI_MODE_VIDEO) safely." which isn't the case at all. > dsi_timing_setup() affects both command mode and video mode panels, and > by no longer having any path that sets bits_per_pclk = 48 (contrary to > the updated code-comment) all DSI DSC panels on SM8350 and above (i.e. > with widebus support) regress thanks to this patch. > > The entire reason that video mode was originally omitted from this code > path is because it was never tested before; any change that enables > widebus for video mode panels should not regress the command mode path. > > Thus add back the path allows 6 bytes or 48 bits to be sent per pclk > on command mode DSI panels with widebus, restoring the panel on devices > like the Sony Xperia 1 III and upwards. > > Fixes: ac47870fd795 ("drm/msm/dsi: fix hdisplay calculation when programming dsi registers") > Signed-off-by: Marijn Suijten > --- > In addition I can't say I understand the original commit message > at all; it mentions a BPC=10 mode however the highest format that > mipi_dsi_pixel_format_to_bpp supports is RGB888 thus it won't > ever return anything above 24, which is the original amount the > non-command-mode path defaulted to (regardless of widebus)... Was that > patch doing anything for video mode at all? > > It feels like the conditional introduced here is only making things more > confusing, but I don't have enough input to confirm what the video-mode > path should be doing in widebus mode (multiply BPC by the number of > components and by 2 in case of widebus?). > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index e8e83ee61eb0..168e37e38fc7 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -1029,10 +1029,14 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > * unused anyway. > */ > h_total -= hdisplay; > - if (wide_bus_enabled) > - bits_per_pclk = mipi_dsi_pixel_format_to_bpp(msm_host->format); > - else > + if (wide_bus_enabled) { > + if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) > + bits_per_pclk = mipi_dsi_pixel_format_to_bpp(msm_host->format); > + else > + bits_per_pclk = 48; > + } else { > bits_per_pclk = 24; > + } Marijn et al, I don't really know this hardware specifically, but I had a very fast look at this patch, and I believe that however you put it, this looks like being either plain wrong or very weird. I think this should be, instead.... /* (don't add this comment) assuming RGB888/666, this will be 24 for both Command and Video modes */ bits_per_pclk = mipi_dsi_pixel_format_to_bpp(msm_host->format); /* Can send twice the bits per clock if WideBus with Video Mode */ if (wide_bus_enabled && msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) bits_per_pclk *= 2; ...because, unless there's a hardware quirk (and that should be really clarified with a big comment stating that), I don't see why command mode should always be 24 bits per clock, and I also don't see why a widebus case would do 48 bits per clock even in the RGB666_PACKED and RGB565 cases (which may not even be possible but *meh*). Just my two cents. Reminding you all again that I don't know this HW, so I may have said something wrong here. Cheers! Angelo > > hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc) * 8, bits_per_pclk); > > > --- > base-commit: 3fa5e5702a82d259897bd7e209469bc06368bf31 > change-id: 20260311-dsi-dsc-regresses-again-4be27dfc4001 > > Best regards,