Received: by 2002:a05:6a10:22f:0:0:0:0 with SMTP id 15csp1624075pxk; Fri, 4 Sep 2020 14:32:33 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxDJwlsP5yLmTlPzz8PvmJqhoXi0OIeBKBCRBgwdAuWqkScUAzpgcsllCE8QAz0MbZOI40o X-Received: by 2002:aa7:de03:: with SMTP id h3mr11063945edv.232.1599255153269; Fri, 04 Sep 2020 14:32:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1599255153; cv=none; d=google.com; s=arc-20160816; b=slBGW9BPM/cw3vUaiXq6RSO7suI4cHKfelwHWYV/seob32piTjMTUnSNqofgI4+VsV nJJXKIvraIe8AMZCyDgLYp23j0pLbm13vaQq8aG7BsB/1e+c6FrtuWr45VtzAa+mGygs KRdjsatx6bwVOMTmibbRX7b1B8o6ee4/UKUqx+y/tFkt1Yj6x3CHlbfCYVkwqR71vOSu cC8yq4fRgvnVGEPB6NcrZB6PU//Y4nyp5RR5OoMGt9v+uRUKP7HqNEB1VDZ/syShYjO6 +0vj7mOKEEPGIen7TfyrT6OVfpeG9pNkMK332VJyPiYVNr9XFmsIbUNBXeIL87kN91q/ WRVw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=lWsIwGs4zwf7QsRg59AIdeA858uxoHmiYKmGxLCL+jc=; b=VxkztEc6BZVprgrofoWtLLi3XL7qHUvVEcNf0tRh09EPfRAD44hyj/wtuIGQPi3omW lA3Zrx0IJ+QwkiXgBryTdBDis26S/QbOjvNc+Vmnn39UeG0wueQJ/7FE8Y1fEA6JtODa WBJsFudxwGidqQkAl5G+VOL+P+HSbK8q3/Ah6WxGiulJUZI15ulw3j/9z/T/EXOnWvYd VFX8/88wSVVUtJm0yrfNrPJ9+CvGY9hKPRxlRtECB6f15cpgWCvorEaTZ7Jjg2JG/c64 HeAdZALq3y5RD6U2lboZnQJY4sBQxilHr9V3H+4mvurHIPU0JAlrTzfwgkd3cMJuMGmH W+eg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=GglQyMIz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r17si5095029ejz.238.2020.09.04.14.32.08; Fri, 04 Sep 2020 14:32:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=GglQyMIz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728076AbgIDVb3 (ORCPT + 99 others); Fri, 4 Sep 2020 17:31:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40644 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727020AbgIDVb2 (ORCPT ); Fri, 4 Sep 2020 17:31:28 -0400 Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 77AA9C061244 for ; Fri, 4 Sep 2020 14:31:26 -0700 (PDT) Received: by mail-wr1-x443.google.com with SMTP id x14so8620793wrl.12 for ; Fri, 04 Sep 2020 14:31:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=lWsIwGs4zwf7QsRg59AIdeA858uxoHmiYKmGxLCL+jc=; b=GglQyMIzcy1VPhXaMWU6sAazP11ZlGD+g1QkZErVQWsYcmle+PPc32oWvxEAfkyFTd Ev/w1OcjerBUY0wPe1XLn+bdaF/xi6AMmdhe5vefEex1oo837qgSi2Hf561OlFMgqOCq 44GZLjptKmZENqtHfEMaze/MDnKS7U7Fb7wc2oIn/WHR795UAnOJsko8SH/WcoNvYf8R wa8fsjD/8w2CDkS+0cjmUIFkDe+GS43tqw0McxBbyeMkqH6eaHQEOGs56qYWpNEhYqFA J63Q8YgnHb4+KS/Ks8AKhs0cMjDj/rlW8mnuD+W//Uz7f7O8/2k8Wj8S1h6S010ZDB1H YVtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=lWsIwGs4zwf7QsRg59AIdeA858uxoHmiYKmGxLCL+jc=; b=YcjStO7DpDxmPfI7rYDDgpgpkmHgRygd81DiZKtGKiFW0XAIzCrYNPceqNDz704EKX qo3iuWpmnEP1Xg2vIrp52Rv4fL+dipkcSCTkZE/CSsnGChVeUIZ9nbAS/E2dKDdmoSg2 dbMWX3SKNQlKZ3E7Hn5qnUp2hMGwcMOyCDZdF1WR64pXmu88XaQ+BtXkvqpmwK30zRhW kq30c9SbcOJUHwCQxeFqGUHA+p0azoyXfSaM7QyBvkj8VSFXaHRNy1Z2QoTRyDiYtJob +9tdm9tnzCnzKUWF50eK/l75cZ+FSuC8ALYLDA4B6R7s44UCzv5ZUcZ2UASL8yqh0yHh JLDQ== X-Gm-Message-State: AOAM533w5lFgWDPClQziWXNfA8L15qgun9zHyJrhxvbCcE0hhl4AlxQ0 sIewq3PE4WJFVob9WpKd6FUM+oAmi0K17A== X-Received: by 2002:a05:6000:1ce:: with SMTP id t14mr8840909wrx.195.1599255085099; Fri, 04 Sep 2020 14:31:25 -0700 (PDT) Received: from smtp.gmail.com (a95-92-181-29.cpe.netcabo.pt. [95.92.181.29]) by smtp.gmail.com with ESMTPSA id z203sm14989819wmc.31.2020.09.04.14.31.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Sep 2020 14:31:24 -0700 (PDT) Date: Fri, 4 Sep 2020 18:31:18 -0300 From: Melissa Wen To: Rodrigo Siqueira Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Brian Starkey , Liviu Dudau , Daniel Vetter , Simon Ser , Leandro Ribeiro , daniels@collabora.com, Emil Velikov Subject: Re: [PATCH v6 2/3] drm/vkms: Compute CRC without change input data Message-ID: <20200904213118.7bpdhokijilb6np3@smtp.gmail.com> References: <20200830142000.146706-1-rodrigosiqueiramelo@gmail.com> <20200830142000.146706-3-rodrigosiqueiramelo@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200830142000.146706-3-rodrigosiqueiramelo@gmail.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/30, Rodrigo Siqueira wrote: > The compute_crc() function is responsible for calculating the > framebuffer CRC value; due to the XRGB format, this function has to > ignore the alpha channel during the CRC computation. Therefore, > compute_crc() set zero to the alpha channel directly in the input > framebuffer, which is not a problem since this function receives a copy > of the original buffer. However, if we want to use this function in a > context without a buffer copy, it will change the initial value. This > patch makes compute_crc() calculate the CRC value without modifying the > input framebuffer. Hi Rodrigo, This commit message no longer reflects the current state of crc computation on vkms, since the alpha channel is no longer ignored (not zeroed) there. I think the point here is to improve readability, which I agree, is it? If so, update this msg. > > Change in V5 (Melissa): > - Rebase and drop bitmap for alpha > Change in V4 (Emil): > - Move bitmap_clear operation and comments to get_pixel function > > Signed-off-by: Rodrigo Siqueira > --- > drivers/gpu/drm/vkms/vkms_composer.c | 34 ++++++++++++++++++---------- > 1 file changed, 22 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c > index f67d1baf1942..c5b32fe5870f 100644 > --- a/drivers/gpu/drm/vkms/vkms_composer.c > +++ b/drivers/gpu/drm/vkms/vkms_composer.c > @@ -9,31 +9,41 @@ > > #include "vkms_drv.h" > > +static u32 get_pixel_from_buffer(int x, int y, const u8 *buffer, > + const struct vkms_composer *composer) > +{ > + u32 pixel; > + int src_offset = composer->offset + (y * composer->pitch) > + + (x * composer->cpp); > + > + pixel = *(u32 *)&buffer[src_offset]; > + > + return pixel; > +} > + > /** > * compute_crc - Compute CRC value on output frame > * > - * @vaddr_out: address to final framebuffer > + * @vaddr: address to final framebuffer > * @composer: framebuffer's metadata > * > * returns CRC value computed using crc32 on the visible portion of > * the final framebuffer at vaddr_out > */ > -static uint32_t compute_crc(void *vaddr_out, struct vkms_composer *composer) > +static uint32_t compute_crc(const u8 *vaddr, > + const struct vkms_composer *composer) > { > - int i, j, src_offset; > + int x, y; > + u32 crc = 0, pixel = 0; > int x_src = composer->src.x1 >> 16; > int y_src = composer->src.y1 >> 16; > int h_src = drm_rect_height(&composer->src) >> 16; > int w_src = drm_rect_width(&composer->src) >> 16; > - u32 crc = 0; > - > - for (i = y_src; i < y_src + h_src; ++i) { > - for (j = x_src; j < x_src + w_src; ++j) { > - src_offset = composer->offset > - + (i * composer->pitch) > - + (j * composer->cpp); > - crc = crc32_le(crc, vaddr_out + src_offset, > - sizeof(u32)); > + > + for (y = y_src; y < y_src + h_src; ++y) { > + for (x = x_src; x < x_src + w_src; ++x) { > + pixel = get_pixel_from_buffer(x, y, vaddr, composer); > + crc = crc32_le(crc, (void *)&pixel, sizeof(u32)); > } > } > > -- > 2.28.0 > Please, update the commit msg. The code improvement look good to me. So: Reviewed-by: Melissa Wen