Received: by 2002:a05:6359:c8b:b0:c7:702f:21d4 with SMTP id go11csp576273rwb; Tue, 4 Oct 2022 07:56:14 -0700 (PDT) X-Google-Smtp-Source: AMsMyM57c8OC306ttksV6CJmCFz89buoCdYm5+dQ1xdcEsc3H5N+Gl7CUudjb8Ij2ZeCCFhJhkPl X-Received: by 2002:a17:907:a05:b0:77b:b538:6476 with SMTP id bb5-20020a1709070a0500b0077bb5386476mr19683552ejc.324.1664895374100; Tue, 04 Oct 2022 07:56:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1664895374; cv=none; d=google.com; s=arc-20160816; b=vqTb0owKxN7m5UehLgfCJJlnR2bb2QHID7A5qb2ljprwutbgVzNfyNuTZzSOPxd6zX bEaO9H4/hOY9q3gVE+gRdVuoslxGgDEbCY9Xh443Z2Hpk6SPBHEqq1OzSOjqVapBUb/c 5o47J4HPDY3o9Wydw0Sjy88QETzQ8N2s37LMQs5DoYjiciXJyJ6ZyEYpMTqKZCWZrQpf Jyp0rQtwUiRyG5oXJF8XM0tQpERqhHyNgUm8Ah6+Vmzxd3qLLbGVXUUzNdJgjx+mCBcN lV4qWGJB1Wtn7nu9vekibSR2UkR2hVz+G9wmP9Y0qI9FmH/7m4ixvkamJLzTJps2C5o7 +oEw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=z41duvCKMloCMs7/yS+eYyEb+WVImktYvIerwjQ5evI=; b=oFxlNUMsgZbwrDOEpyaUk5Ko75Hfp+6sDrHDHTnMqX7jOLVNym8dWGaDJ9G3nnJ8ff 1FYF99+LQzH6Vk70+3cOhpervYgXHvCPd6GsGzOTDVDZn/xJ4hDGrZLEGEtxkf28mjGC ua9eaHA3gSQEwL+RIlJsXOCPM+1HZgfG1Uj+aSpxsy25vc91TKOMQc0jnTpp541HDd9u 72cnO2pMf9rm35SJUVr8Rzder2dpwK2jXuu6HjYAFBsVsgXyMxvTgNnWJ+EL04R0843x 6H3UX/xHocRQxeBPJCKJcdQEern5N8egneAC/avTO+8N48YfwMkFcOu0ChDsgSI6KSTR pOCQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=j6jR0OTm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b4-20020a0564021f0400b00458b4bebee5si7609649edb.406.2022.10.04.07.55.49; Tue, 04 Oct 2022 07:56:14 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=j6jR0OTm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229832AbiJDOlY (ORCPT + 99 others); Tue, 4 Oct 2022 10:41:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46660 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229819AbiJDOlU (ORCPT ); Tue, 4 Oct 2022 10:41:20 -0400 Received: from mail-yb1-xb2d.google.com (mail-yb1-xb2d.google.com [IPv6:2607:f8b0:4864:20::b2d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0540D13F5D for ; Tue, 4 Oct 2022 07:41:19 -0700 (PDT) Received: by mail-yb1-xb2d.google.com with SMTP id e20so2524712ybh.2 for ; Tue, 04 Oct 2022 07:41:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date; bh=z41duvCKMloCMs7/yS+eYyEb+WVImktYvIerwjQ5evI=; b=j6jR0OTmSljAbwsij5gsdUL+4ZFD75ZNHeSNzLJ6oPjvuO064edahqdHCjca2FVoNE BMIHxdhPWGZSeyUjkE1RHfGXRB1XY9mBdsQfsg8uEzypRG2oXvwJJoxkKG1IpkFp+9Vt GB2IpJ0heualwYLJrYc4+9+qsVEaYfxxVwtG4s6CPJMrVT3z67fuJVcjqBlA7iF/Tvmq 169kZ+gBTNZftLrhpsg1ijx/7QF/AcuUd6OcGh1vUK0r5nE/7O6Z+j4DRK85NvpKEiM+ r/M3ChmBXywKUVwIWfnXmH9FcDntC5jTRgrPSDSEn/mZh9kSoOL/A0e2vegGLeFvFokX 5m6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date; bh=z41duvCKMloCMs7/yS+eYyEb+WVImktYvIerwjQ5evI=; b=yXEvJSTsNp+VoergTC5tHLCeVsOsPMjGWm8MyHPdp5PzrtK+mPdzivkpH4UPqW8/WL ZcrWrQsbrpo+VIcaO5HPxl0R+8x63fr5DCPfl8ziw3l7xOVNyPons33sM9zDf/biT6V2 /LpCtsdnrH14pHcVWG/HO3JIin12470VLvEhPqiBLflUNaXIbuPjbdm8TAPNS+Dny13S ftq/aMjVNeTvHzEcIkaDroSxGwoYLOf0bgWWw88QQzjdUjoMr7BFY9IL6U5cm3cKINuB liS+OliWDSpvIvzJCfFkUUmMH+IDM4+dKmH4y043msPxb7H9LBl2g/iNZV02TrZ1K79/ LYng== X-Gm-Message-State: ACrzQf1bip7/lOSsf6EEtrT5tCMd7vcKnm2bdMhNFTpYf7k/Gbz+r1yp CMDXd+zYPYo+mZ18Kzk5DeRmhULN2tWrYiaFIxP+IQ== X-Received: by 2002:a25:2e4e:0:b0:6bc:ac92:a4df with SMTP id b14-20020a252e4e000000b006bcac92a4dfmr26652384ybn.153.1664894478194; Tue, 04 Oct 2022 07:41:18 -0700 (PDT) MIME-Version: 1.0 References: <20221001190807.358691-1-marijn.suijten@somainline.org> <20221001190807.358691-6-marijn.suijten@somainline.org> <20221001202313.fkdsv5ul4v6akhc3@SoMainline.org> In-Reply-To: <20221001202313.fkdsv5ul4v6akhc3@SoMainline.org> From: Dmitry Baryshkov Date: Tue, 4 Oct 2022 17:41:07 +0300 Message-ID: Subject: Re: [PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields To: Marijn Suijten , phone-devel@vger.kernel.org, Rob Clark , Dmitry Baryshkov , Vinod Koul , ~postmarketos/upstreaming@lists.sr.ht, AngeloGioacchino Del Regno , Konrad Dybcio , Martin Botka , Jami Kettunen , David Airlie , Daniel Vetter , Abhinav Kumar , Sean Paul , Thomas Zimmermann , Javier Martinez Canillas , Alex Deucher , Douglas Anderson , Vladimir Lypak , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, Lyude Paul Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 1 Oct 2022 at 23:23, Marijn Suijten wrote: > > On 2022-10-01 21:08:07, Marijn Suijten wrote: > > msm's dsi_host specifies negative BPG offsets which fill the full 8 bits > > of a char thanks to two's complement: this however results in those bits > > bleeding into the next parameter when the field is only expected to > > contain 6-bit wide values. > > As a consequence random slices appear corrupted on-screen (tested on a > > Sony Tama Akatsuki device with sdm845). > > > > Use AND operators to limit all values that constitute the RC Range > > parameter fields to their expected size. > > > > Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data") > > Signed-off-by: Marijn Suijten > > --- > > drivers/gpu/drm/display/drm_dsc_helper.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c > > index c869c6e51e2b..2e7ef242685d 100644 > > --- a/drivers/gpu/drm/display/drm_dsc_helper.c > > +++ b/drivers/gpu/drm/display/drm_dsc_helper.c > > @@ -243,11 +243,11 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, > > */ > > for (i = 0; i < DSC_NUM_BUF_RANGES; i++) { > > pps_payload->rc_range_parameters[i] = > > - cpu_to_be16((dsc_cfg->rc_range_params[i].range_min_qp << > > + cpu_to_be16(((dsc_cfg->rc_range_params[i].range_min_qp & 0x1f) << > > DSC_PPS_RC_RANGE_MINQP_SHIFT) | > > - (dsc_cfg->rc_range_params[i].range_max_qp << > > + ((dsc_cfg->rc_range_params[i].range_max_qp & 0x1f) << > > DSC_PPS_RC_RANGE_MAXQP_SHIFT) | > > - (dsc_cfg->rc_range_params[i].range_bpg_offset)); > > + (dsc_cfg->rc_range_params[i].range_bpg_offset & 0x3f)); > > Pre-empting the reviews: I was contemplating whether to use FIELD_PREP > here, given that it's not yet used anywhere else in this file. For that > I'd remove the existing _SHIFT definitions and replace them with: > > #define DSC_PPS_RC_RANGE_MINQP_MASK GENMASK(15, 11) > #define DSC_PPS_RC_RANGE_MAXQP_MASK GENMASK(10, 6) > #define DSC_PPS_RC_RANGE_BPG_OFFSET_MASK GENMASK(5, 0) > > And turn this section of code into: > > cpu_to_be16(FIELD_PREP(DSC_PPS_RC_RANGE_MINQP_MASK, > dsc_cfg->rc_range_params[i].range_min_qp) | > FIELD_PREP(DSC_PPS_RC_RANGE_MAXQP_MASK, > dsc_cfg->rc_range_params[i].range_max_qp) | > FIELD_PREP(DSC_PPS_RC_RANGE_BPG_OFFSET_MASK, > dsc_cfg->rc_range_params[i].range_bpg_offset)); > > Is that okay/recommended? This is definitely easier to review. However if you do not want to use FIELD_PREP, it would be better to split this into a series of `data |= something` assignments terminated with the rc_range_parameters[i] assignment. > > - Marijn > > > } > > > > /* PPS 88 */ > > -- > > 2.37.3 > > -- With best wishes Dmitry