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 169C31093172 for ; Fri, 20 Mar 2026 03:55:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3B7AF10E193; Fri, 20 Mar 2026 03:55:34 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; secure) header.d=pm.me header.i=@pm.me header.b="UmkTdwkn"; dkim-atps=neutral Received: from mail-4316.protonmail.ch (mail-4316.protonmail.ch [185.70.43.16]) by gabe.freedesktop.org (Postfix) with ESMTPS id DC07210E193; Fri, 20 Mar 2026 03:55:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pm.me; s=protonmail3; t=1773978929; x=1774238129; bh=6hSHoAw+FRg4yq7oAQNcLdcehITdZuUHNxQc1UYr+tQ=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=UmkTdwknZC4PQKwvcBvl6lE8BO7HtecVoEMgnk0Xa+PTjHeN7ir0CEbRHH8H+vevr lzJKtK7aLpp6GOkGf0FXEid0USpYtzZraxtfLKY9pVvqzTKnGkETKaJAOpvmxaDrQV i/Mf0YiEor6qIklVuiaiSIvyNf0hDVWitncFYAKQ0bDKbaQ8J6Gunsb9zl0essO7u0 rM2fSWEAT1C1aWsDwLM+O6H9wBvweJgr4DYssDPUotXHcigWsBGuoBmE3Mhx3tCOqO 6YPQnzH2zvJp8QuthcdgqoK/0SPEekqPKd+QJ4vFyR7N2UtZG5UzKSg2Mj8R9+vJUB gf3kduvyQ2XKw== Date: Fri, 20 Mar 2026 03:55:24 +0000 To: Dmitry Baryshkov From: Alexander Koskovich Cc: Jonathan Marek , Neil Armstrong , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , Jeffrey Hugo , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org Subject: Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation Message-ID: In-Reply-To: References: <20260319-dsi-rgb101010-support-v3-0-85b99df2d090@pm.me> <20260319-dsi-rgb101010-support-v3-4-85b99df2d090@pm.me> <1360a31d-669e-48df-a1be-f0af4a253cd7@linaro.org> <3gLK4s97giqqXagfHKhfiIHbfbl2snwfOj9dcTNGPUYi10w9-1EdATqzz1LPCVTpz4bLFYOm8u_Fl8PfC7t5yabows4UCzRKVwjraEWW6hc=@pm.me> <3f8763af-aad2-4d92-90c8-cfd290212503@linaro.org> <7fb9dd9d-13f9-7bba-93d1-08f42dd6ee38@marek.ca> Feedback-ID: 37836894:user:proton X-Pm-Message-ID: b39faaa2045b0f5a2a636019996afd901f86c9aa MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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" On Thursday, March 19th, 2026 at 9:45 PM, Dmitry Baryshkov wrote: > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote: > > On 3/19/26 10:54 AM, Neil Armstrong wrote: > > > On 3/19/26 15:40, Alexander Koskovich wrote: > > > > On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong > > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > On 3/19/26 12:58, Alexander Koskovich wrote: > > > > > > Using bits_per_component * 3 as the divisor for the compressed = INTF > > > > > > timing width produces constant FIFO errors for the BOE BF068MWM= -TD0 > > > > > > panel due to bits_per_component being 10 which results in a div= isor > > > > > > of 30 instead of 24. > > > > > > > > > > > > Regardless of the compression ratio and pixel depth, 24 bits of > > > > > > compressed data are transferred per pclk, so the divisor should > > > > > > always be 24. > > > > > > > > > > Not true with widebus, specify why 24 and because DSI widebus is > > > > > not implemented yet. > > > > > > > > > DSI widebus is implemented, and always used when available. The adjustm= ent > > for DSI widebus just doesn't happen in this function. > > > > > > > > > > > > > > Signed-off-by: Alexander Koskovich > > > > > > --- > > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 +++= +----- > > > > > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git > > > > > > a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > > > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > > > > > index 0ba777bda253..5419ef0be137 100644 > > > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > > > > > @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_param= s( > > > > > > } > > > > > > > > > > > > /* > > > > > > - * for DSI, if compression is enabled, then divide the > > > > > > horizonal active > > > > > > - * timing parameters by compression ratio. bits of 3 > > > > > > components(R/G/B) > > > > > > - * is compressed into bits of 1 pixel. > > > > > > + * For DSI, if DSC is enabled, 24 bits of compressed data = are > > > > > > + * transferred per pclk regardless of the source pixel dep= th. > > > > > > */ > > > > > > if (phys_enc->hw_intf->cap->type !=3D INTF_DP && > > > > > > timing->compression_en) { > > > > > > struct drm_dsc_config *dsc =3D > > > > > > dpu_encoder_get_dsc_config(phys_enc->parent); > > > > > > + > > > > > Drop this change > > > > > > > > > > > /* > > > > > > * TODO: replace drm_dsc_get_bpp_int with logic to h= andle > > > > > > * fractional part if there is fraction > > > > > > */ > > > > > > - timing->width =3D timing->width * drm_dsc_get_bpp_int(= dsc) / > > > > > > - (dsc->bits_per_component * 3); > > > > > > + timing->width =3D timing->width * drm_dsc_get_bpp_int(= dsc) / 24; > > > > > > > > > > It would be helpful to somehow show that 24 is 8 * 3, 8 being > > > > > the byte width and 3 the compression ratio. > > > > > > > > Can you clarify what the 3 represents? My panel should have a 3.75:= 1 > > > > compression > > > > ratio (30/8) so the final divisor of 24 would be wrong for my panel > > > > if its the > > > > compression ratio? > > > > > > So my guess is that while the exact ratio on the DSI lanes is 3.75:1, > > > the ratio > > > used to calculate the INTF timings is 3, then the DSC encoder probabl= y > > > does the > > > magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes. > > > > > > > That's not how it works. INTF (which feeds DSI) is after DSC compressio= n. > > > > INTF timings are always in RGB888 (24-bit) units. Ignoring widebus deta= ils, > > the INTF timing should match what is programmed on the DSI side (hdispl= ay, > > which is calculated as bytes per line / 3). > > > > (fwiw, the current "timing->width =3D ..." calculation here blames to m= e, but > > what I wrote originally was just "timing->width =3D timing->width / 3" = with a > > comment about being incomplete.) > > > Okay. After reading the docs (sorry, it took a while). >=20 > - When widebus is not enabled, the transfer is always 24 bit of > compressed data. Thus if it is not in play, pclk and timing->width > should be scaled by source_pixel_depth / compression_ratio / 24. In > case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'. >=20 > For RGB101010 / 8bpp DSC this should result in the PCLK being lowered > by the factor of 3 (=3D 24 / (30 / 3.75)) >=20 > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes > more than 24 bits. In this case the PCLK and timing->width should be > scaled exactly by the DSC compression ratio, which is > 'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component). >=20 > So, this piece of code needs to be adjusted to check for the widebus > being enabled or not. >=20 Modified drm_mode_to_intf_timing_params & dsi_adjust_pclk_for_compression t= o account for widebus, but the hdisplay I get is different from stock with = widebus factored in. Getting 288 instead of 360 now which produces constant= FIFO errors. This is with widebus enabled, using 3 * dsc->bits_per_component. Should it = be 24 regardless of widebus? > > > In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in > > > account bits_per_component, > > > so I presume the actual DSI pclk _is_ timing->width * > > > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3), > > > which is your 3.75:1, but the INTF needs to generate timing->width * > > > drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels > > > to the DSC encoder which will emit timing->width * > > > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels. > > > > > > > The hdisplay calculation in dsi_adjust_pclk_for_compression (which only > > affects the clock rate) seems to be wrong, and I think Alexander's pane= l > > must be running at a 20% lower clock because of it. dsi_timing_setup ha= s the > > right hdisplay adjustment. >=20 > That function also needs to be adjusted accordingly. I think only the > dsi_timing_setup() is correct at this point. Note, widebus / not-widebus > cases should be handled separately. >=20 > > > In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the > > > INTF side. >=20 > In this case DSC compression ratio is 3.75, so it's not true. >=20 > -- > With best wishes > Dmitry >=20 Thanks, Alex