Re: [PATCH v2 4/8] accel/qaic: Add control path

From: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
To: Jeffrey Hugo <quic_jhugo@quicinc.com>
Cc: ogabbay@kernel.org, airlied@gmail.com, daniel@ffwll.ch,
	dri-devel@lists.freedesktop.org,
	jacek.lawrynowicz@linux.intel.com, quic_pkanojiy@quicinc.com,
	quic_carlv@quicinc.com, quic_ajitpals@quicinc.com,
	linux-doc@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v2 4/8] accel/qaic: Add control path
Date: Wed, 1 Mar 2023 14:18:16 +0100	[thread overview]
Message-ID: <20230301131816.GC3815307@linux.intel.com> (raw)
In-Reply-To: <1675698105-19025-5-git-send-email-quic_jhugo@quicinc.com>

On Mon, Feb 06, 2023 at 08:41:41AM -0700, Jeffrey Hugo wrote:
> +	 * It turns out several of the iommu drivers don't combine adjacent
> +	 * regions, which is really what we expect based on the description of
> +	 * dma_map_sgtable(), so lets see if that can be done.  It makes our message
> +	 * more efficent.
> +	 */

Interesting ...

> +	last = sgt->sgl;
> +	nents_dma = nents;
> +	size = QAIC_MANAGE_EXT_MSG_LENGTH - msg_hdr_len - sizeof(*out_trans);
> +	for_each_sgtable_sg(sgt, sg, i) {
> +		if (sg_dma_address(last) + sg_dma_len(last) !=
> +		    sg_dma_address(sg)) {
> +			size -= sizeof(*asp);
> +			/* Save 1K for possible follow-up transactions. */
> +			if (size < SZ_1K) {
> +				nents_dma = i;
> +				break;
> +			}
> +		}
> +		last = sg;
> +	}

I would say there is reason why iommu do not combine or there is problem in iommu driver.
If there is reason for not-combining this code is wrong.
If problem is in iommu driver why not fixup iommu ?

And why not create sg list that don't have adjacent entries in the first place ?

Regards
Stanislaw