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 X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A27C7C433E2 for ; Tue, 8 Sep 2020 08:50:26 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 493E0215A4 for ; Tue, 8 Sep 2020 08:50:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="DnZH3pe1"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="hvxn9skr" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 493E0215A4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=T9lDqHH0lBl9dqRmhY/b9OFk8WdyrklSskWbMpPGkEs=; b=DnZH3pe18/ySC0AMoesBR4xXv vmWfqi4Nt8O7xscsHy48S/Is5xUcfvCu79UOYyye5J1qibUPlf85tDcF2dBZkme1Y4QF4ujVPkMo4 zNXMk96ho29RarqzqBS0PBM+s+Dy7xdj4wflUq/3zMqxbldkZfxsb/8h49ESOmzj6Rw2QECzzS5Cw FH+JPEjeNSGZnmzV6wXcmTa4rThmznU8MDCqpHwQSSUl6GrZMbT/emb9MuWGz5ybtuJqvWPaZZk2n 9rGDTY9vvJSv0xtI7e+4+p39X1Dcs2ZvawrGwXY3Uuq1UFXIlge7mW6IBA+fON81Yd3Fr+Dad64UI I0tOD5swQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFZJG-00014M-Ij; Tue, 08 Sep 2020 08:49:02 +0000 Received: from mail-wr1-x442.google.com ([2a00:1450:4864:20::442]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFZJD-00012m-1r for linux-arm-kernel@lists.infradead.org; Tue, 08 Sep 2020 08:49:00 +0000 Received: by mail-wr1-x442.google.com with SMTP id z1so18245823wrt.3 for ; Tue, 08 Sep 2020 01:48:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=vTwZpb2SRbHS6TVjDRxWxiFRdUx1UorL5y3KysCor9o=; b=hvxn9skrMRRr0q95ExVeGBXDKL50YXqPxTkDw/op5wnsECwbMgTPjRembThpSe3EYK HAdhN+caBKBO/UMSUpqVSK6hrPlnx0e9fN4VMOPkmrB5/nj3AI8Gdf5cdpkPoAgCJhGX wLDN/wIwneGsAHwiuvEdIk/6G5aE0Y4MBD780= 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 :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=vTwZpb2SRbHS6TVjDRxWxiFRdUx1UorL5y3KysCor9o=; b=Obo4/Tr9FfKAuLuq1NkW5P1kTye78tryY3nST2pGx8gZ6VOjOqby/sABQ9Ad3lhdj5 GUPn0eDdlBYv2YDLrhpwnsbwiLe95r0zPk0bcuwYU6AsDPC1viMimUgbC0+S+BlnEr3O KA7SsVtmKPTctzQTVFs7/QLgzK/Nd50Y5X36oT48Zj4BxrhsDYJfxdDFtgI4KqmeHmk0 OeMeSuVDbShhLmscHfXh2ikJjs7pSy/KOa6n4AVehvSU3vuCT1OFj2uH+RUwUIM5XB+I mkDF/b681h0/iaWTn+0rPebR49WEBxMXCvjn46YmAGswUHxnwBuxEkojcaZoKHAyz1Nb Kx1g== X-Gm-Message-State: AOAM532QNgFcOZbqrPvU4EvwNb+tD2Hd25FKfBb0sWa7P8oj7/tiGwK4 /zX3YhBQUjw/1EZtPGdHuqgHN+YsFsTILy6s X-Google-Smtp-Source: ABdhPJyChK39vZ43On41Fya27UnXYFaJZY862924QCXz3b7+wz4vQQztCQcIMrRa95whcLZtQ6e+NQ== X-Received: by 2002:adf:9125:: with SMTP id j34mr28207058wrj.157.1599554937849; Tue, 08 Sep 2020 01:48:57 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:57f4:0:efd0:b9e5:5ae6:c2fa]) by smtp.gmail.com with ESMTPSA id r14sm33235979wrn.56.2020.09.08.01.48.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Sep 2020 01:48:57 -0700 (PDT) Date: Tue, 8 Sep 2020 10:48:55 +0200 From: Daniel Vetter To: Tomi Valkeinen Subject: Re: [PATCH] drm: mxsfb: check framebuffer pitch Message-ID: <20200908084855.GH2352366@phenom.ffwll.local> Mail-Followup-To: Tomi Valkeinen , Stefan Agner , Laurent Pinchart , jsarha@ti.com, marex@denx.de, airlied@linux.ie, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20200907160343.124405-1-stefan@agner.ch> <20200907161712.GF6047@pendragon.ideasonboard.com> <20200907181855.GE2352366@phenom.ffwll.local> <86615b4b1551d4a6f1cfcc13b38e616c@agner.ch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 5.7.0-1-amd64 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200908_044859_135751_D5EAF5E7 X-CRM114-Status: GOOD ( 33.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: marex@denx.de, Stefan Agner , kernel@pengutronix.de, dri-devel@lists.freedesktop.org, airlied@linux.ie, festevam@gmail.com, s.hauer@pengutronix.de, linux-kernel@vger.kernel.org, jsarha@ti.com, Laurent Pinchart , daniel@ffwll.ch, shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org, linux-imx@nxp.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 08, 2020 at 11:18:25AM +0300, Tomi Valkeinen wrote: > Hi, > > On 08/09/2020 10:55, Stefan Agner wrote: > > On 2020-09-07 20:18, Daniel Vetter wrote: > >> On Mon, Sep 07, 2020 at 07:17:12PM +0300, Laurent Pinchart wrote: > >>> Hi Stefan, > >>> > >>> Thank you for the patch. > >>> > >>> On Mon, Sep 07, 2020 at 06:03:43PM +0200, Stefan Agner wrote: > >>>> The lcdif IP does not support a framebuffer pitch (stride) other than > >>>> the CRTC width. Check for equality and reject the state otherwise. > >>>> > >>>> This prevents a distorted picture when using 640x800 and running the > >>>> Mesa graphics stack. Mesa tires to use a cache aligned stride, which > >>> > >>> s/tires/tries/ > >>> > >>>> leads at that particular resolution to width != stride. Currently > >>>> Mesa has no fallback behavior, but rejecting this configuration allows > >>>> userspace to handle the issue correctly. > >>> > >>> I'm increasingly impressed by how featureful this IP core is :-) > >>> > >>>> Signed-off-by: Stefan Agner > >>>> --- > >>>> drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++++++++++++++++++---- > >>>> 1 file changed, 18 insertions(+), 4 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > >>>> index b721b8b262ce..79aa14027f91 100644 > >>>> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > >>>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > >>>> @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct drm_plane *plane, > >>>> { > >>>> struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev); > >>>> struct drm_crtc_state *crtc_state; > >>>> + unsigned int pitch; > >>>> + int ret; > >>>> > >>>> crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, > >>>> &mxsfb->crtc); > >>>> > >>>> - return drm_atomic_helper_check_plane_state(plane_state, crtc_state, > >>>> - DRM_PLANE_HELPER_NO_SCALING, > >>>> - DRM_PLANE_HELPER_NO_SCALING, > >>>> - false, true); > >>>> + ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state, > >>>> + DRM_PLANE_HELPER_NO_SCALING, > >>>> + DRM_PLANE_HELPER_NO_SCALING, > >>>> + false, true); > >>>> + if (ret || !plane_state->visible) > >>> > >>> Would it be more explict to check for !plane_state->fb ? Otherwise I'll > >>> have to verify that !fb always implies !visible :-) > >>> > >>>> + return ret; > >>>> + > >>>> + pitch = crtc_state->mode.hdisplay * > >>>> + plane_state->fb->format->cpp[0]; > >>> > >>> This holds on a single line. > >>> > >>>> + if (plane_state->fb->pitches[0] != pitch) { > >>>> + dev_err(plane->dev->dev, > >>>> + "Invalid pitch: fb and crtc widths must be the same"); > >>> > >>> I'd turn this into a dev_dbg(), printing error messages to the kernel > >>> log in response to user-triggered conditions is a bit too verbose and > >>> could flood the log. > >>> > >>> Wouldn't it be best to catch this issue when creating the framebuffer ? > >> > >> Yeah this should be verified at addfb time. We try to validate as early as > >> possible. > >> -Daniel > >> > > > > Sounds sensible. From what I can tell fb_create is the proper callback > > to implement this at addfb time. Will give this a try. > > > > FWIW, I got the idea from drivers/gpu/drm/tilcdc/tilcdc_plane.c. Maybe > > should be moved to addfb there too? > > But you don't know the crtc width when creating the framebuffer. Hm right this is a different check. What we could check in fb_create for both is that the logical fb size matches exactly the pitch. That's not sufficient criteria, but it will at least catch some of them already. But yeah we'd need both here. -Daniel > > Tomi > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel