Re: [PATCH 04/17] media: atomisp: pci: do not use err var when checking port validity for ISP2400
From: Andy Shevchenko
Date: Mon Nov 01 2021 - 15:28:51 EST
On Mon, Nov 1, 2021 at 9:07 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> On 11/1/21 15:10, Mauro Carvalho Chehab wrote:
...
> Testing BYT support definitely is on my radar. Note that BYT
> also has the additional issue that the atomisp2 on BYT can be
> enumerated as either a PCI dev (which may work) or an ACPI/platform
> dev which is unsupported in the original atomisp2 code-drop and
> seems non trivial (because of pci config space writes) to get to
> work.
>
> On the T100TA the device is actually enumerated as an ACPI/platform
> device and the BIOS option to change this is hidden. In the mean
> time I've gained quite a lot of experience with changing hidden
> BIOS options though, so I can easily put it in PCI mode for
> testing. But eventually we also need to tackle ACPI enumeration
> support...
Not sure if you saw my very far from being even tested patch...
Share it here just in case. I believe you will have a better idea on
how to implement it.
--
With Best Regards,
Andy Shevchenko
From 8250354d9faf5ccb21a26b1d8a7a771db24467e5 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Date: Fri, 3 Nov 2017 21:39:47 +0200
Subject: [PATCH 1/1] i915: Enumerate AtomISP when it's an ACPI device
When AtomISP is an ACPI subdevice of graphics, we need to enumerate it
in the similar way how it's done for LPE Audio, because interrupt line
is provided via GPU register.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/Makefile | 3 +
drivers/gpu/drm/i915/i915_drv.h | 13 ++
drivers/gpu/drm/i915/i915_irq.c | 3 +
drivers/gpu/drm/i915/intel_isp.c | 244 +++++++++++++++++++++++++++++++
4 files changed, 263 insertions(+)
create mode 100644 drivers/gpu/drm/i915/intel_isp.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 660bb03de6fc..ef5d578031c1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -317,6 +317,9 @@ i915-y += intel_gvt.o
include $(src)/gvt/Makefile
endif
+# AtomISP for VLV and CHT
+i915-y += intel_isp.o
+
obj-$(CONFIG_DRM_I915) += i915.o
obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 12256218634f..75971879f617 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1236,6 +1236,14 @@ struct drm_i915_private {
int irq;
} lpe_audio;
+ /* necessary resource sharing with AtomISP driver */
+ struct {
+ bool found;
+ struct resource mmio;
+ struct platform_device *platdev;
+ int irq;
+ } isp;
+
struct i915_pmu pmu;
struct i915_hdcp_comp_master *hdcp_master;
@@ -1962,6 +1970,11 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
bool trampoline);
#define I915_CMD_PARSER_TRAMPOLINE_SIZE 8
+/* intel_isp.c */
+int intel_isp_init(struct drm_i915_private *dev_priv);
+void intel_isp_teardown(struct drm_i915_private *dev_priv);
+void intel_isp_irq_handler(struct drm_i915_private *dev_priv);
+
/* intel_device_info.c */
static inline struct intel_device_info *
mkwrite_device_info(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 77680bca46ee..6e71a86b8584 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1707,6 +1707,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
I915_LPE_PIPE_B_INTERRUPT))
intel_lpe_audio_irq_handler(dev_priv);
+ if (iir & I915_ISP_INTERRUPT)
+ intel_isp_irq_handler(dev_priv);
+
/*
* VLV_IIR is single buffered, and reflects the level
* from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last.
diff --git a/drivers/gpu/drm/i915/intel_isp.c b/drivers/gpu/drm/i915/intel_isp.c
new file mode 100644
index 000000000000..42134a001930
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_isp.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Author: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
+ */
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+
+#include "i915_drv.h"
+
+#define HAS_ISP(dev_priv) ((dev_priv)->isp.platdev != NULL)
+
+static struct platform_device *
+isp_platdev_create(struct drm_i915_private *dev_priv)
+{
+ struct platform_device_info pinfo = {};
+ struct platform_device *platdev;
+ struct resource rsc[2];
+
+ rsc[0].start = rsc[0].end = dev_priv->isp.irq;
+ rsc[0].flags = IORESOURCE_IRQ;
+ rsc[0].name = "isp-irq";
+
+ rsc[1].start = dev_priv->isp.mmio.start;
+ rsc[1].end = dev_priv->isp.mmio.end;
+ rsc[1].flags = IORESOURCE_MEM;
+ rsc[1].name = "isp-mmio";
+
+ pinfo.name = "atomisp-isp2";
+ pinfo.id = -1;
+ pinfo.res = rsc;
+ pinfo.num_res = ARRAY_SIZE(rsc);
+ pinfo.dma_mask = DMA_BIT_MASK(32);
+
+ platdev = platform_device_register_full(&pinfo);
+ if (IS_ERR(platdev))
+ DRM_ERROR("Failed to allocate ISP platform device\n");
+
+ return platdev;
+}
+
+static void isp_platdev_destroy(struct drm_i915_private *dev_priv)
+{
+ /* XXX Note that platform_device_register_full() allocates a dma_mask
+ * and never frees it. We can't free it here as we cannot guarantee
+ * this is the last reference (i.e. that the dma_mask will not be
+ * used after our unregister). So ee choose to leak the sizeof(u64)
+ * allocation here - it should be fixed in the platform_device rather
+ * than us fiddle with its internals.
+ */
+
+ platform_device_unregister(dev_priv->isp.platdev);
+}
+
+static void isp_irq_unmask(struct irq_data *d)
+{
+}
+
+static void isp_irq_mask(struct irq_data *d)
+{
+}
+
+static struct irq_chip isp_irqchip = {
+ .name = "isp_irqchip",
+ .irq_mask = isp_irq_mask,
+ .irq_unmask = isp_irq_unmask,
+};
+
+static int isp_irq_init(struct drm_i915_private *dev_priv)
+{
+ int irq = dev_priv->isp.irq;
+
+ WARN_ON(!intel_irqs_enabled(dev_priv));
+ irq_set_chip_and_handler_name(irq,
+ &isp_irqchip,
+ handle_simple_irq,
+ "isp_irq_handler");
+
+ return irq_set_chip_data(irq, dev_priv);
+}
+
+static acpi_status
+isp_detect(acpi_handle handle, u32 level, void *context, void **return_value)
+{
+ struct drm_i915_private *dev_priv = context;
+ struct list_head resource_list;
+ struct resource_entry *entry;
+ struct acpi_device *adev;
+ unsigned long long adr;
+ acpi_status status;
+ int ret;
+
+ if (dev_priv->isp.found)
+ return AE_OK;
+
+ status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
+ if (ACPI_FAILURE(status))
+ return AE_OK;
+
+ /* The _ADR must match the one of supported ISP */
+ if (!IS_VALLEYVIEW(dev_priv) || adr != 0x0f38)
+ return AE_OK;
+
+ if (acpi_bus_get_device(handle, &adev))
+ return AE_OK;
+
+ /* Then fill MMIO resource if any */
+ INIT_LIST_HEAD(&resource_list);
+ ret = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
+ if (ret < 0)
+ return AE_OK;
+
+ resource_list_for_each_entry(entry, &resource_list) {
+ if (resource_type(entry->res) == IORESOURCE_MEM) {
+ dev_priv->isp.mmio = *entry->res;
+ break;
+ }
+ }
+
+ acpi_dev_free_resource_list(&resource_list);
+
+ dev_priv->isp.found = true;
+ return AE_OK;
+}
+
+static int isp_setup(struct drm_i915_private *dev_priv)
+{
+ int ret;
+
+ dev_priv->isp.irq = irq_alloc_desc(0);
+ if (dev_priv->isp.irq < 0) {
+ DRM_ERROR("Failed to allocate IRQ desc: %d\n", dev_priv->isp.irq);
+ ret = dev_priv->isp.irq;
+ goto err;
+ }
+
+ DRM_DEBUG("irq = %d\n", dev_priv->isp.irq);
+
+ ret = isp_irq_init(dev_priv);
+
+ if (ret) {
+ DRM_ERROR("Failed to initialize irqchip for ISP: %d\n", ret);
+ goto err_free_irq;
+ }
+
+ dev_priv->isp.platdev = isp_platdev_create(dev_priv);
+
+ if (IS_ERR(dev_priv->isp.platdev)) {
+ ret = PTR_ERR(dev_priv->isp.platdev);
+ DRM_ERROR("Failed to create ISP platform device: %d\n", ret);
+ goto err_free_irq;
+ }
+
+ return 0;
+
+err_free_irq:
+ irq_free_desc(dev_priv->isp.irq);
+err:
+ dev_priv->isp.irq = -1;
+ dev_priv->isp.platdev = NULL;
+ return ret;
+}
+
+/**
+ * intel_isp_irq_handler() - forwards the ISP IRQ
+ * @dev_priv: the i915 drm device private data
+ *
+ * The ISP IRQ is forwarded to the IRQ handler registered by AtomISP
+ * driver.
+ */
+void intel_isp_irq_handler(struct drm_i915_private *dev_priv)
+{
+ int ret;
+
+ if (!HAS_ISP(dev_priv))
+ return;
+
+ ret = generic_handle_irq(dev_priv->isp.irq);
+ if (ret)
+ DRM_ERROR_RATELIMITED("error handling ISP irq: %d\n", ret);
+}
+
+/**
+ * intel_isp_init() - detect and setup the bridge between AtomISP driver and i915
+ * @dev_priv: the i915 drm device private data
+ *
+ * Return: 0 if successful. non-zero if detection or
+ * location/initialization fails
+ */
+int intel_isp_init(struct drm_i915_private *dev_priv)
+{
+ struct device *dev = dev_priv->drm.dev;
+ int ret = -ENODEV;
+
+ if (has_acpi_companion(dev))
+ acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_HANDLE(dev), 1,
+ isp_detect, NULL, dev_priv, NULL);
+ if (dev_priv->isp.found) {
+ ret = isp_setup(dev_priv);
+ if (ret < 0)
+ DRM_ERROR("failed to setup ISP bridge\n");
+ }
+ return ret;
+}
+
+/**
+ * intel_isp_teardown() - destroy the bridge between AtomISP driver and i915
+ * @dev_priv: the i915 drm device private data
+ *
+ * release all the resources for AtomISP <-> i915 bridge.
+ */
+void intel_isp_teardown(struct drm_i915_private *dev_priv)
+{
+ if (!HAS_ISP(dev_priv))
+ return;
+
+ isp_platdev_destroy(dev_priv);
+
+ irq_free_desc(dev_priv->isp.irq);
+
+ dev_priv->isp.found = false;
+}
--
2.33.0