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=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 985CCC4338F for ; Wed, 28 Jul 2021 07:16:10 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id F22DE60F41 for ; Wed, 28 Jul 2021 07:16:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F22DE60F41 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 733586E3F5; Wed, 28 Jul 2021 07:16:09 +0000 (UTC) Received: from mx1.smtp.larsendata.com (mx1.smtp.larsendata.com [91.221.196.215]) by gabe.freedesktop.org (Postfix) with ESMTPS id C37686E3F5 for ; Wed, 28 Jul 2021 07:16:07 +0000 (UTC) Received: from mail01.mxhotel.dk (mail01.mxhotel.dk [91.221.196.236]) by mx1.smtp.larsendata.com (Halon) with ESMTPS id b0815579-ef73-11eb-9082-0050568c148b; Wed, 28 Jul 2021 07:16:12 +0000 (UTC) Received: from ravnborg.org (80-162-45-141-cable.dk.customer.tdc.net [80.162.45.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: sam@ravnborg.org) by mail01.mxhotel.dk (Postfix) with ESMTPSA id 85CB7194B83; Wed, 28 Jul 2021 09:16:26 +0200 (CEST) Date: Wed, 28 Jul 2021 09:16:03 +0200 X-Report-Abuse-To: abuse@mxhotel.dk From: Sam Ravnborg To: Anitha Chrisanthus Subject: Re: [PATCH 04/14] drm/kmb : W/A for 256B cache alignment for video Message-ID: References: <20210728003126.1425028-1-anitha.chrisanthus@intel.com> <20210728003126.1425028-4-anitha.chrisanthus@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210728003126.1425028-4-anitha.chrisanthus@intel.com> 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: , Cc: edmund.j.dea@intel.com, dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Anitha, On Tue, Jul 27, 2021 at 05:31:16PM -0700, Anitha Chrisanthus wrote: > For B0 silicon, the media driver pads the decoded video dmabufs for 256B > alignment. This is the backing buffer of the framebuffer and info in the > drm frame buffer is not correct for these buffers as this is done > internally in the media driver. This change extracts the meta data info > from dmabuf priv structure and uses that info for programming stride and > offsets in kmb_plane_atomic_update(). > > Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display") > Signed-off-by: Edmund Dea > Signed-off-by: Anitha Chrisanthus Drop extra space in subject before ':' > --- > drivers/gpu/drm/kmb/kmb_drv.h | 1 + > drivers/gpu/drm/kmb/kmb_plane.c | 38 ++++++++++++++++++++--- > drivers/gpu/drm/kmb/kmb_vidmem.h | 52 ++++++++++++++++++++++++++++++++ > 3 files changed, 87 insertions(+), 4 deletions(-) > create mode 100644 drivers/gpu/drm/kmb/kmb_vidmem.h > > diff --git a/drivers/gpu/drm/kmb/kmb_drv.h b/drivers/gpu/drm/kmb/kmb_drv.h > index ebbaa5f422d5..0904e6eb2a09 100644 > --- a/drivers/gpu/drm/kmb/kmb_drv.h > +++ b/drivers/gpu/drm/kmb/kmb_drv.h > @@ -49,6 +49,7 @@ struct kmb_drm_private { > int kmb_under_flow; > int kmb_flush_done; > int layer_no; > + struct viv_vidmem_metadata *md_info; I cannot see this member used in this patch - can it be dropped? > }; > > static inline struct kmb_drm_private *to_kmb(const struct drm_device *dev) > diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c > index 2888dd5dcc2c..e45419d6ed96 100644 > --- a/drivers/gpu/drm/kmb/kmb_plane.c > +++ b/drivers/gpu/drm/kmb/kmb_plane.c > @@ -11,12 +11,16 @@ > #include > #include > #include > +#include > #include > #include > > +#include > + > #include "kmb_drv.h" > #include "kmb_plane.h" > #include "kmb_regs.h" > +#include "kmb_vidmem.h" > > const u32 layer_irqs[] = { > LCD_INT_VL0, > @@ -294,8 +298,10 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > unsigned int ctrl = 0, val = 0, out_format = 0; > unsigned int src_w, src_h, crtc_x, crtc_y; > unsigned char plane_id; > - int num_planes; > + int num_planes, i; > static dma_addr_t addr[MAX_SUB_PLANES]; > + struct viv_vidmem_metadata *md = NULL; > + struct drm_gem_object *gem_obj; > > if (!plane || !new_plane_state || !old_plane_state) > return; > @@ -325,6 +331,16 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > drm_dbg(&kmb->drm, > "src_w=%d src_h=%d, fb->format->format=0x%x fb->flags=0x%x\n", > src_w, src_h, fb->format->format, fb->flags); > + gem_obj = drm_gem_fb_get_obj(fb, plane_id); > + if (gem_obj && gem_obj->import_attach && > + gem_obj->import_attach->dmabuf && > + gem_obj->import_attach->dmabuf->priv) { > + md = gem_obj->import_attach->dmabuf->priv; > + > + /* Check if metadata is coming from hantro driver */ > + if (md->magic != HANTRO_IMAGE_VIV_META_DATA_MAGIC) > + md = NULL; > + } > > width = fb->width; > height = fb->height; > @@ -332,6 +348,11 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > drm_dbg(&kmb->drm, "dma_len=%d ", dma_len); > kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN(plane_id), dma_len); > kmb_write_lcd(kmb, LCD_LAYERn_DMA_LEN_SHADOW(plane_id), dma_len); > + if (md) { > + for (i = 0; i < 3; i++) > + fb->pitches[i] = md->plane[i].stride; > + } > + > kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_VSTRIDE(plane_id), > fb->pitches[0]); > kmb_write_lcd(kmb, LCD_LAYERn_DMA_LINE_WIDTH(plane_id), > @@ -339,18 +360,22 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > > addr[Y_PLANE] = drm_fb_cma_get_gem_addr(fb, new_plane_state, 0); > kmb_write_lcd(kmb, LCD_LAYERn_DMA_START_ADDR(plane_id), > - addr[Y_PLANE] + fb->offsets[0]); > + addr[Y_PLANE]); > val = get_pixel_format(fb->format->format); > val |= get_bits_per_pixel(fb->format); > /* Program Cb/Cr for planar formats */ > if (num_planes > 1) { > kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_VSTRIDE(plane_id), > - width * fb->format->cpp[0]); > + fb->pitches[1]); > kmb_write_lcd(kmb, LCD_LAYERn_DMA_CB_LINE_WIDTH(plane_id), > (width * fb->format->cpp[0])); > > addr[U_PLANE] = drm_fb_cma_get_gem_addr(fb, new_plane_state, > U_PLANE); > + if (md) { > + addr[U_PLANE] += md->plane[1].offset - > + (addr[U_PLANE] - addr[Y_PLANE]); > + } I failed to follow why: 1) offsets is no logner needed 2) If pitches is always set or only set with a hantro buffer 3) Why addr[U_PLANE] is assigned twice in the md != NULL case > /* check if Cb/Cr is swapped*/ > if (num_planes == 3 && (val & LCD_LAYER_CRCB_ORDER)) > kmb_write_lcd(kmb, > @@ -364,7 +389,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > if (num_planes == 3) { > kmb_write_lcd(kmb, > LCD_LAYERn_DMA_CR_LINE_VSTRIDE(plane_id), > - ((width) * fb->format->cpp[0])); > + fb->pitches[2]); > > kmb_write_lcd(kmb, > LCD_LAYERn_DMA_CR_LINE_WIDTH(plane_id), > @@ -373,6 +398,11 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > addr[V_PLANE] = drm_fb_cma_get_gem_addr(fb, > new_plane_state, > V_PLANE); > + if (md) { > + addr[V_PLANE] += > + md->plane[2].offset - > + (addr[V_PLANE] - addr[Y_PLANE]); > + } Likewise - is pitches always valid and why assing addr[V_PLANE] twice? > > /* check if Cb/Cr is swapped*/ > if (val & LCD_LAYER_CRCB_ORDER) > diff --git a/drivers/gpu/drm/kmb/kmb_vidmem.h b/drivers/gpu/drm/kmb/kmb_vidmem.h > new file mode 100644 > index 000000000000..06198d413f50 > --- /dev/null > +++ b/drivers/gpu/drm/kmb/kmb_vidmem.h > @@ -0,0 +1,52 @@ > +/* SPDX-License-Identifier: GPL-2.0-only > + * > + * Copyright © 2018-2020 Intel Corporation > + */ > + > +#ifndef __KMB_VIDMEM_H__ > +#define __KMB_VIDMEM_H__ > + > +#define HANTRO_MAGIC(ch0, ch1, ch2, ch3) \ > + ((unsigned long)(unsigned char)(ch0) | \ > + ((unsigned long)(unsigned char)(ch1) << 8) | \ > + ((unsigned long)(unsigned char)(ch2) << 16) | \ > + ((unsigned long)(unsigned char)(ch3) << 24)) ... This header looks like it belongs outside the drm driver - I assume the hantro driver needs this? Or is this some uapi stuff? Sam