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 6A55CC021A7 for ; Thu, 13 Feb 2025 06:31:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 324DA10E246; Thu, 13 Feb 2025 06:31:17 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="apIga0x1"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3620210E246 for ; Thu, 13 Feb 2025 06:31:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1739428276; x=1770964276; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=GeThfe2Wds0od4rB2sv//Gc4NP8au/H6bnUyR81ZJSY=; b=apIga0x1/uvdWtJHyygqyEqeoUb256LKDb1EIuwEEN/2kUkBBz1LrUr1 S/D2Vq/mA4b2XcWihNdiRk/GMDfsZ8fvs9cO3RpeycRx8eGxxrmZNnbjM 4GjILhGJ9WY5s93tZts2o/2X5TxF4LteXX8cVd4ofPeoKf+P6yfnedAHI kZn+tNH0A81vi1hZ+aQTBfkRBMSl2cEr7SXEW1arWSuFm9kspOPKef32b ovKtK4+ORP9GMfOvx46yt+fyQrv7R8SUab34NkQRjxT1pfcHXRWh1ug1r CjBlAy2an6vFm3uMfCyD8Kgmb3/VNPvYN+vqARbvc+aKo9tbnNTvfESQg g==; X-CSE-ConnectionGUID: iiLc0z5NQzezGCZPkdzeVw== X-CSE-MsgGUID: PrwMtiV7QNaLLCIYtc5YWQ== X-IronPort-AV: E=McAfee;i="6700,10204,11343"; a="39815717" X-IronPort-AV: E=Sophos;i="6.13,282,1732608000"; d="scan'208";a="39815717" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by orvoesa112.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Feb 2025 22:31:16 -0800 X-CSE-ConnectionGUID: Efw18okKSy6zsvpSo+Dntw== X-CSE-MsgGUID: 6jfnTJSJREKWQq1UzkV1PQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,282,1732608000"; d="scan'208";a="143892448" Received: from orsosgc001.jf.intel.com (HELO orsosgc001.intel.com) ([10.165.21.142]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Feb 2025 22:31:16 -0800 Date: Wed, 12 Feb 2025 22:31:15 -0800 Message-ID: <85tt8y73v0.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Harish Chegondi Cc: Subject: Re: [PATCH v9 5/8] drm/xe/eustall: Add support to handle dropped EU stall data In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-redhat-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Mon, 10 Feb 2025 05:46:46 -0800, Harish Chegondi wrote: > Hi Harish, > If the user space doesn't read the EU stall data fast enough, > it is possible that the EU stall data buffer can get filled, > and if the hardware wants to write more data, it simply drops > data due to unavailable buffer space. In that case, hardware > sets a bit in a register. If the driver detects data drop, > the driver read() returns -EIO error to let the user space > know that HW has dropped data. The -EIO error is returned > even if there is EU stall data in the buffer. A subsequent > read by the user space returns the remaining EU stall data. > > v9: Move all data drop handling code to this patch Good, separating out makes this easier to review. I would actually make this the last patch, but anyway it's ok as is too. > Clear all drop data bits before returning -EIO. > > Signed-off-by: Harish Chegondi > --- > drivers/gpu/drm/xe/xe_eu_stall.c | 39 ++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/gpu/drm/xe/xe_eu_stall.c b/drivers/gpu/drm/xe/xe_eu_stall.c > index 53f17aac7d3b..428267010805 100644 > --- a/drivers/gpu/drm/xe/xe_eu_stall.c > +++ b/drivers/gpu/drm/xe/xe_eu_stall.c > @@ -53,6 +53,10 @@ struct xe_eu_stall_data_stream { > struct xe_gt *gt; > struct xe_bo *bo; > struct per_xecore_buf *xecore_buf; > + struct { > + bool reported_to_user; > + xe_dss_mask_t mask; > + } data_drop; > struct delayed_work buf_poll_work; > struct workqueue_struct *buf_poll_wq; > }; > @@ -331,12 +335,24 @@ static bool eu_stall_data_buf_poll(struct xe_eu_stall_data_stream *stream) > if (num_data_rows(total_data) >= stream->wait_num_reports) > min_data_present = true; > } > + if (write_ptr_reg & XEHPC_EUSTALL_REPORT_OVERFLOW_DROP) > + set_bit(xecore, stream->data_drop.mask); > xecore_buf->write = write_ptr; > mutex_unlock(&xecore_buf->ptr_lock); > } > return min_data_present; > } > > +static void clear_dropped_eviction_line_bit(struct xe_gt *gt, u16 group, u16 instance) > +{ > + u32 write_ptr_reg; > + > + /* On PVC, the overflow bit has to be cleared by writing 1 to it. */ > + write_ptr_reg = _MASKED_BIT_ENABLE(XEHPC_EUSTALL_REPORT_OVERFLOW_DROP); > + > + xe_gt_mcr_unicast_write(gt, XEHPC_EUSTALL_REPORT, write_ptr_reg, group, instance); > +} > + > static int xe_eu_stall_data_buf_read(struct xe_eu_stall_data_stream *stream, > char __user *buf, size_t count, > size_t *total_data_size, struct xe_gt *gt, > @@ -436,6 +452,22 @@ static ssize_t xe_eu_stall_stream_read_locked(struct xe_eu_stall_data_stream *st > unsigned int xecore; > int ret = 0; > > + if (bitmap_weight(stream->data_drop.mask, XE_MAX_DSS_FUSE_BITS)) { > + if (!stream->data_drop.reported_to_user) { > + for_each_dss_steering(xecore, gt, group, instance) { > + if (test_bit(xecore, stream->data_drop.mask)) { > + clear_dropped_eviction_line_bit(gt, group, instance); > + clear_bit(xecore, stream->data_drop.mask); > + } > + } This is not making any sense. How can we clear_dropped_eviction_line_bit before reading the data? The HW will set it right back up. At least the code in the previous version made some sense. So we should at least go back to that and review that. Though it also had issue with how many times -EIO is returned etc. The other issue is the locking. As I suggested in my comments on Patch 4/8, I don't think we need a per xecore_buf lock, just one ptr_lock for reading all the DSS's. And then we can use that here too. But at least let's get something sane for review first. > + stream->data_drop.reported_to_user = true; > + xe_gt_dbg(gt, "EU stall data dropped in XeCores: %*pb\n", > + XE_MAX_DSS_FUSE_BITS, stream->data_drop.mask); > + return -EIO; > + } > + stream->data_drop.reported_to_user = false; > + } > + > for_each_dss_steering(xecore, gt, group, instance) { > ret = xe_eu_stall_data_buf_read(stream, buf, count, &total_size, > gt, group, instance, xecore); > @@ -457,6 +489,7 @@ static ssize_t xe_eu_stall_stream_read_locked(struct xe_eu_stall_data_stream *st > * before calling read(). > * > * Returns: The number of bytes copied or a negative error code on failure. > + * -EIO if HW drops any EU stall data when the buffer is full. > */ > static ssize_t xe_eu_stall_stream_read(struct file *file, char __user *buf, > size_t count, loff_t *ppos) > @@ -543,6 +576,9 @@ static int xe_eu_stall_stream_enable(struct xe_eu_stall_data_stream *stream) > > for_each_dss_steering(xecore, gt, group, instance) { > write_ptr_reg = xe_gt_mcr_unicast_read(gt, XEHPC_EUSTALL_REPORT, group, instance); > + /* Clear any drop bits set and not cleared in the previous session. */ > + if (write_ptr_reg & XEHPC_EUSTALL_REPORT_OVERFLOW_DROP) > + clear_dropped_eviction_line_bit(gt, group, instance); > write_ptr = REG_FIELD_GET(XEHPC_EUSTALL_REPORT_WRITE_PTR_MASK, write_ptr_reg); > read_ptr_reg = REG_FIELD_PREP(XEHPC_EUSTALL_REPORT1_READ_PTR_MASK, write_ptr); > read_ptr_reg = _MASKED_FIELD(XEHPC_EUSTALL_REPORT1_READ_PTR_MASK, read_ptr_reg); > @@ -554,6 +590,9 @@ static int xe_eu_stall_stream_enable(struct xe_eu_stall_data_stream *stream) > xecore_buf->write = write_ptr; > xecore_buf->read = write_ptr; > } > + stream->data_drop.reported_to_user = false; > + bitmap_zero(stream->data_drop.mask, XE_MAX_DSS_FUSE_BITS); > + > reg_value = _MASKED_FIELD(EUSTALL_MOCS | EUSTALL_SAMPLE_RATE, > REG_FIELD_PREP(EUSTALL_MOCS, gt->mocs.uc_index << 1) | > REG_FIELD_PREP(EUSTALL_SAMPLE_RATE, > -- > 2.48.1 >