| Message ID | 7609686.EvYhyI6sBW@kreacher |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: scan: MIPI DiSco for Imaging support | expand |
Hi Rafael, On subject: s/DiSco/DisCo/ On Fri, Oct 20, 2023 at 04:39:27PM +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Add information extracted from the MIPI DiSco for Imaging device Ditto. > properties to software nodes created during the CSI-2 connection graph > discovery. > > Link: https://www.mipi.org/specifications/mipi-di This URL is broken. The correct URL is: https://www.mipi.org/specifications/mipi-disco-imaging > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/internal.h | 1 > drivers/acpi/mipi-di.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++- How about mipi-disco.c? I wouldn't mind having mipi-disco-imaging.c either. > drivers/acpi/scan.c | 12 ++ > include/acpi/acpi_bus.h | 10 ++ > 4 files changed, 259 insertions(+), 4 deletions(-) > > Index: linux-pm/drivers/acpi/mipi-di.c > =================================================================== > --- linux-pm.orig/drivers/acpi/mipi-di.c > +++ linux-pm/drivers/acpi/mipi-di.c > @@ -6,12 +6,16 @@ > * > * Support MIPI DisCo for Imaging by parsing ACPI _CRS CSI-2 records defined in > * Section 6.4.3.8.2.4 "Camera Serial Interface (CSI-2) Connection Resource > - * Descriptor" of ACPI 6.5. > + * Descriptor" of ACPI 6.5 and using device properties defined by the MIPI DisCo > + * for Imaging specification. > * > * The implementation looks for the information in the ACPI namespace (CSI-2 > * resource descriptors in _CRS) and constructs software nodes compatible with > * Documentation/firmware-guide/acpi/dsd/graph.rst to represent the CSI-2 > - * connection graph. > + * connection graph. The software nodes are then populated with the data > + * extracted from the _CRS CSI-2 resource descriptors and the MIPI DisCo > + * for Imaging device properties present in _DSD for the ACPI device objects > + * with CSI-2 connections. > */ > > #include <linux/acpi.h> > @@ -434,6 +438,238 @@ void acpi_mipi_scan_crs_csi2(void) > prepare_crs_csi2_swnodes(csi2); > } > > +/* > + * Get the index of the next property in the property array, with a given > + * maximum value. > + */ > +#define NEXT_PROPERTY(index, max) \ > + (WARN_ON((index) > ACPI_DEVICE_SWNODE_##max) ? \ > + ACPI_DEVICE_SWNODE_##max : (index)++) > + > +static void init_csi2_port_local(struct acpi_device *adev, > + struct acpi_device_software_node_port *port, > + struct fwnode_handle *port_fwnode, > + unsigned int index) > +{ > + acpi_handle handle = acpi_device_handle(adev); > + unsigned int num_link_freqs; > + int ret; > + > + ret = fwnode_property_count_u64(port_fwnode, "mipi-img-link-frequencies"); > + if (ret <= 0) > + return; > + > + num_link_freqs = ret; > + if (num_link_freqs > ARRAY_SIZE(port->link_frequencies)) { > + acpi_handle_info(handle, "Too many link frequencies: %u\n", > + num_link_freqs); > + num_link_freqs = ARRAY_SIZE(port->link_frequencies); > + } > + > + ret = fwnode_property_read_u64_array(port_fwnode, > + "mipi-img-link-frequencies", > + port->link_frequencies, > + num_link_freqs); > + if (ret) { > + acpi_handle_info(handle, "Unable to get link frequencies (%d)\n", > + ret); > + return; > + } > + > + port->ep_props[NEXT_PROPERTY(index, EP_LINK_FREQUENCIES)] = > + PROPERTY_ENTRY_U64_ARRAY_LEN("link-frequencies", > + port->link_frequencies, > + num_link_freqs); > +} > + > +static void init_csi2_port(struct acpi_device *adev, > + struct acpi_device_software_nodes *swnodes, > + struct acpi_device_software_node_port *port, > + struct fwnode_handle *port_fwnode, > + unsigned int port_index) > +{ > + unsigned int ep_prop_index = ACPI_DEVICE_SWNODE_EP_CLOCK_LANES; > + acpi_handle handle = acpi_device_handle(adev); > + u8 val[ARRAY_SIZE(port->data_lanes)]; > + unsigned int num_lanes = 0; > + int ret; > + > + if (GRAPH_PORT_NAME(port->port_name, port->port_nr)) > + return; > + > + swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)] = > + SOFTWARE_NODE(port->port_name, port->port_props, > + &swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT]); > + > + ret = fwnode_property_read_u8(port_fwnode, "mipi-img-clock-lane", val); > + if (!ret) > + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_CLOCK_LANES)] = > + PROPERTY_ENTRY_U32("clock-lanes", val[0]); > + > + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-data-lanes"); > + if (ret > 0) { > + num_lanes = ret; > + > + if (num_lanes > ARRAY_SIZE(port->data_lanes)) { > + acpi_handle_info(handle, "Too many data lanes: %u\n", > + num_lanes); > + num_lanes = ARRAY_SIZE(port->data_lanes); > + } > + > + ret = fwnode_property_read_u8_array(port_fwnode, > + "mipi-img-data-lanes", > + val, num_lanes); > + if (!ret) { > + unsigned int i; > + > + for (i = 0; i < num_lanes; i++) > + port->data_lanes[i] = val[i]; > + > + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_DATA_LANES)] = > + PROPERTY_ENTRY_U32_ARRAY_LEN("data-lanes", > + port->data_lanes, > + num_lanes); > + } > + } > + > + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities"); > + if (ret > 0) { > + unsigned long mask; > + unsigned int i; > + > + /* > + * Total number of lanes here is clock lane + data lanes. > + * Require that number to be low enough so they all can be > + * covered by the bits in one byte. > + */ > + BUILD_BUG_ON(BITS_PER_TYPE(u8) <= ARRAY_SIZE(port->data_lanes)); > + > + fwnode_property_read_u8_array(port_fwnode, > + "mipi-img-lane-polarities", > + val, 1); > + > + for (mask = val[0], i = 0; i < num_lanes + 1; i++) > + port->lane_polarities[i] = test_bit(i, &mask); This works only up to seven lanes. I guess this can be addressed later in case we get devices with 8 lanes. But adding a comment here (or where the maximum number of lanes is defined) that this needs to be fixed would be prudent to avoid introducing a bug later on. > + > + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_LANE_POLARITIES)] = > + PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities", > + port->lane_polarities, > + 1 + num_lanes); > + } else { > + acpi_handle_info(handle, "No lane polarity bytes\n"); > + } > + > + swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] = > + SOFTWARE_NODE("endpoint@0", swnodes->ports[port_index].ep_props, > + &swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)]); > + > + if (port->crs_csi2_local) > + init_csi2_port_local(adev, port, port_fwnode, ep_prop_index); > +} > + > +#define MIPI_IMG_PORT_PREFIX "mipi-img-port-" > + > +static struct fwnode_handle *get_mipi_port_handle(struct fwnode_handle *adev_fwnode, > + unsigned int port_nr) > +{ > + char port_name[sizeof(MIPI_IMG_PORT_PREFIX) + 2]; > + > + if (snprintf(port_name, sizeof(port_name), "%s%u", > + MIPI_IMG_PORT_PREFIX, port_nr) >= sizeof(port_name)) > + return NULL; > + > + return fwnode_get_named_child_node(adev_fwnode, port_name); > +} > + > +static void init_crs_csi2_swnodes(struct crs_csi2 *csi2) > +{ > + struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER }; > + struct acpi_device_software_nodes *swnodes = csi2->swnodes; > + acpi_handle handle = csi2->handle; > + struct fwnode_handle *adev_fwnode; > + struct acpi_device *adev; > + acpi_status status; > + unsigned int i; > + int ret; > + > + /* > + * Bail out if the swnodes are not available (either they have not been > + * allocated or they have been assigned to the device already). > + */ > + if (!swnodes) > + return; > + > + adev = acpi_fetch_acpi_dev(handle); > + if (!adev) > + return; > + > + adev_fwnode = acpi_fwnode_handle(adev); > + > + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > + if (ACPI_FAILURE(status)) { > + acpi_handle_info(handle, "Unable to get the path name\n"); > + return; > + } > + > + swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT] = > + SOFTWARE_NODE(buffer.pointer, swnodes->dev_props, NULL); > + > + for (i = 0; i < swnodes->num_ports; i++) { > + struct acpi_device_software_node_port *port = &swnodes->ports[i]; > + struct fwnode_handle *port_fwnode; > + > + /* > + * The MIPI DisCo for Imaging specification defines _DSD device > + * properties for providing CSI-2 port parameters that can be > + * accessed through the generic device properties framework. To > + * access them, it is first necessary to find the data node > + * representing the port under the given ACPI device object. > + */ > + port_fwnode = get_mipi_port_handle(adev_fwnode, port->port_nr); > + if (!port_fwnode) { > + acpi_handle_info(handle, > + "MIPI port name too long for port %u\n", > + port->port_nr); > + continue; > + } > + > + init_csi2_port(adev, swnodes, port, port_fwnode, i); > + > + fwnode_handle_put(port_fwnode); > + } > + > + ret = software_node_register_node_group(swnodes->nodeptrs); > + if (ret < 0) { > + acpi_handle_info(handle, > + "Unable to register software nodes (%d)\n", ret); > + return; > + } > + > + adev->swnodes = swnodes; > + adev_fwnode->secondary = software_node_fwnode(swnodes->nodes); > + > + /* > + * Prevents the swnodes from this csi2 entry from being assigned again > + * or freed prematurely. > + */ > + csi2->swnodes = NULL; > +} > + > +/** > + * acpi_mipi_init_crs_csi2_swnodes - Initialize _CRS CSI-2 software nodes > + * > + * Use MIPI DiSco for Imaging device properties to finalize the initialization > + * of CSI-2 software nodes for all ACPI device objects that have been already > + * enumerated. > + */ > +void acpi_mipi_init_crs_csi2_swnodes(void) > +{ > + struct crs_csi2 *csi2, *csi2_tmp; > + > + list_for_each_entry_safe(csi2, csi2_tmp, &acpi_mipi_crs_csi2_list, entry) > + init_crs_csi2_swnodes(csi2); > +} > + > /** > * acpi_mipi_crs_csi2_cleanup - Free _CRS CSI-2 temporary data > */ > Index: linux-pm/drivers/acpi/internal.h > =================================================================== > --- linux-pm.orig/drivers/acpi/internal.h > +++ linux-pm/drivers/acpi/internal.h > @@ -287,6 +287,7 @@ static inline void acpi_init_lpit(void) > > void acpi_mipi_check_crs_csi2(acpi_handle handle); > void acpi_mipi_scan_crs_csi2(void); > +void acpi_mipi_init_crs_csi2_swnodes(void); > void acpi_mipi_crs_csi2_cleanup(void); > > #endif /* _ACPI_INTERNAL_H_ */ > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -2447,6 +2447,13 @@ static void acpi_scan_postponed_branch(a > > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > acpi_bus_check_add_2, NULL, NULL, (void **)&adev); > + > + /* > + * Populate the ACPI _CRS CSI-2 software nodes for the ACPI devices that > + * have been added above. > + */ > + acpi_mipi_init_crs_csi2_swnodes(); > + > acpi_bus_attach(adev, NULL); > } > > @@ -2516,11 +2523,12 @@ int acpi_bus_scan(acpi_handle handle) > return -ENODEV; > > /* > - * Allocate ACPI _CRS CSI-2 software nodes using information extracted > + * Set up ACPI _CRS CSI-2 software nodes using information extracted > * from the _CRS CSI-2 resource descriptors during the ACPI namespace > - * walk above. > + * walk above and MIPI DiSco for Imaging device properties. > */ > acpi_mipi_scan_crs_csi2(); > + acpi_mipi_init_crs_csi2_swnodes(); > > acpi_bus_attach(device, (void *)true); > > Index: linux-pm/include/acpi/acpi_bus.h > =================================================================== > --- linux-pm.orig/include/acpi/acpi_bus.h > +++ linux-pm/include/acpi/acpi_bus.h > @@ -366,10 +366,17 @@ struct acpi_device_data { > > struct acpi_gpio_mapping; > > +#define ACPI_DEVICE_SWNODE_ROOT 0 > + > #define ACPI_DEVICE_CSI2_DATA_LANES 4 > > #define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH 8 > > +enum acpi_device_swnode_dev_props { > + ACPI_DEVICE_SWNODE_DEV_NUM_OF, > + ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES > +}; > + > enum acpi_device_swnode_port_props { > ACPI_DEVICE_SWNODE_PORT_REG, > ACPI_DEVICE_SWNODE_PORT_NUM_OF, > @@ -425,12 +432,14 @@ struct acpi_device_software_node_port { > > /** > * struct acpi_device_software_nodes - Software nodes for an ACPI device > + * @dev_props: Device properties. > * @nodes: Software nodes for root as well as ports and endpoints. > * @nodeprts: Array of software node pointers, for (un)registering them. > * @ports: Information related to each port and endpoint within a port. > * @num_ports: The number of ports. > */ > struct acpi_device_software_nodes { > + struct property_entry dev_props[ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES]; > struct software_node *nodes; > const struct software_node **nodeptrs; > struct acpi_device_software_node_port *ports; > @@ -455,6 +464,7 @@ struct acpi_device { > struct acpi_device_data data; > struct acpi_scan_handler *handler; > struct acpi_hotplug_context *hp; > + struct acpi_device_software_nodes *swnodes; > const struct acpi_gpio_mapping *driver_gpios; > void *driver_data; > struct device dev; > > >
Hi Sakari, On Tue, Oct 31, 2023 at 11:33 AM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Rafael, > > On subject: > > s/DiSco/DisCo/ Huh Thanks for catching this! > On Fri, Oct 20, 2023 at 04:39:27PM +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Add information extracted from the MIPI DiSco for Imaging device > > Ditto. > > > properties to software nodes created during the CSI-2 connection graph > > discovery. > > > > Link: https://www.mipi.org/specifications/mipi-di > > This URL is broken. The correct URL is: > > https://www.mipi.org/specifications/mipi-disco-imaging OK > > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/acpi/internal.h | 1 > > drivers/acpi/mipi-di.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++- > > How about mipi-disco.c? I wouldn't mind having mipi-disco-imaging.c either. Well, mipi-disco-imaging is a bit too long and the meaning of mipi-disco would not be clear IMO. What about mipi-disco-img? [cut] > > + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities"); > > + if (ret > 0) { > > + unsigned long mask; > > + unsigned int i; > > + > > + /* > > + * Total number of lanes here is clock lane + data lanes. > > + * Require that number to be low enough so they all can be > > + * covered by the bits in one byte. > > + */ > > + BUILD_BUG_ON(BITS_PER_TYPE(u8) <= ARRAY_SIZE(port->data_lanes)); > > + > > + fwnode_property_read_u8_array(port_fwnode, > > + "mipi-img-lane-polarities", > > + val, 1); > > + > > + for (mask = val[0], i = 0; i < num_lanes + 1; i++) > > + port->lane_polarities[i] = test_bit(i, &mask); > > This works only up to seven lanes. True, but then ACPI_DEVICE_CSI2_DATA_LANES is defined as 4. > I guess this can be addressed later in > case we get devices with 8 lanes. But adding a comment here (or where the > maximum number of lanes is defined) that this needs to be fixed would be > prudent to avoid introducing a bug later on. The comment above talks about this, but I can add one more comment next to the ACPI_DEVICE_CSI2_DATA_LANES definition. > > + > > + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_LANE_POLARITIES)] = > > + PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities", > > + port->lane_polarities, > > + 1 + num_lanes); > > + } else { > > + acpi_handle_info(handle, "No lane polarity bytes\n"); > > + } > > + > > + swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] = > > + SOFTWARE_NODE("endpoint@0", swnodes->ports[port_index].ep_props, > > + &swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)]); > > + > > + if (port->crs_csi2_local) > > + init_csi2_port_local(adev, port, port_fwnode, ep_prop_index); > > +} > > + > > +#define MIPI_IMG_PORT_PREFIX "mipi-img-port-" > > + > > +static struct fwnode_handle *get_mipi_port_handle(struct fwnode_handle *adev_fwnode, > > + unsigned int port_nr) > > +{ > > + char port_name[sizeof(MIPI_IMG_PORT_PREFIX) + 2]; > > + > > + if (snprintf(port_name, sizeof(port_name), "%s%u", > > + MIPI_IMG_PORT_PREFIX, port_nr) >= sizeof(port_name)) > > + return NULL; > > + > > + return fwnode_get_named_child_node(adev_fwnode, port_name); > > +} > > + > > +static void init_crs_csi2_swnodes(struct crs_csi2 *csi2) > > +{ > > + struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER }; > > + struct acpi_device_software_nodes *swnodes = csi2->swnodes; > > + acpi_handle handle = csi2->handle; > > + struct fwnode_handle *adev_fwnode; > > + struct acpi_device *adev; > > + acpi_status status; > > + unsigned int i; > > + int ret; > > + > > + /* > > + * Bail out if the swnodes are not available (either they have not been > > + * allocated or they have been assigned to the device already). > > + */ > > + if (!swnodes) > > + return; > > + > > + adev = acpi_fetch_acpi_dev(handle); > > + if (!adev) > > + return; > > + > > + adev_fwnode = acpi_fwnode_handle(adev); > > + > > + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); > > + if (ACPI_FAILURE(status)) { > > + acpi_handle_info(handle, "Unable to get the path name\n"); > > + return; > > + } > > + > > + swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT] = > > + SOFTWARE_NODE(buffer.pointer, swnodes->dev_props, NULL); > > + > > + for (i = 0; i < swnodes->num_ports; i++) { > > + struct acpi_device_software_node_port *port = &swnodes->ports[i]; > > + struct fwnode_handle *port_fwnode; > > + > > + /* > > + * The MIPI DisCo for Imaging specification defines _DSD device > > + * properties for providing CSI-2 port parameters that can be > > + * accessed through the generic device properties framework. To > > + * access them, it is first necessary to find the data node > > + * representing the port under the given ACPI device object. > > + */ > > + port_fwnode = get_mipi_port_handle(adev_fwnode, port->port_nr); > > + if (!port_fwnode) { > > + acpi_handle_info(handle, > > + "MIPI port name too long for port %u\n", > > + port->port_nr); > > + continue; > > + } > > + > > + init_csi2_port(adev, swnodes, port, port_fwnode, i); > > + > > + fwnode_handle_put(port_fwnode); > > + } > > + > > + ret = software_node_register_node_group(swnodes->nodeptrs); > > + if (ret < 0) { > > + acpi_handle_info(handle, > > + "Unable to register software nodes (%d)\n", ret); > > + return; > > + } > > + > > + adev->swnodes = swnodes; > > + adev_fwnode->secondary = software_node_fwnode(swnodes->nodes); > > + > > + /* > > + * Prevents the swnodes from this csi2 entry from being assigned again > > + * or freed prematurely. > > + */ > > + csi2->swnodes = NULL; > > +} > > + > > +/** > > + * acpi_mipi_init_crs_csi2_swnodes - Initialize _CRS CSI-2 software nodes > > + * > > + * Use MIPI DiSco for Imaging device properties to finalize the initialization > > + * of CSI-2 software nodes for all ACPI device objects that have been already > > + * enumerated. > > + */ > > +void acpi_mipi_init_crs_csi2_swnodes(void) > > +{ > > + struct crs_csi2 *csi2, *csi2_tmp; > > + > > + list_for_each_entry_safe(csi2, csi2_tmp, &acpi_mipi_crs_csi2_list, entry) > > + init_crs_csi2_swnodes(csi2); > > +} > > + > > /** > > * acpi_mipi_crs_csi2_cleanup - Free _CRS CSI-2 temporary data > > */ > > Index: linux-pm/drivers/acpi/internal.h > > =================================================================== > > --- linux-pm.orig/drivers/acpi/internal.h > > +++ linux-pm/drivers/acpi/internal.h > > @@ -287,6 +287,7 @@ static inline void acpi_init_lpit(void) > > > > void acpi_mipi_check_crs_csi2(acpi_handle handle); > > void acpi_mipi_scan_crs_csi2(void); > > +void acpi_mipi_init_crs_csi2_swnodes(void); > > void acpi_mipi_crs_csi2_cleanup(void); > > > > #endif /* _ACPI_INTERNAL_H_ */ > > Index: linux-pm/drivers/acpi/scan.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/scan.c > > +++ linux-pm/drivers/acpi/scan.c > > @@ -2447,6 +2447,13 @@ static void acpi_scan_postponed_branch(a > > > > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, > > acpi_bus_check_add_2, NULL, NULL, (void **)&adev); > > + > > + /* > > + * Populate the ACPI _CRS CSI-2 software nodes for the ACPI devices that > > + * have been added above. > > + */ > > + acpi_mipi_init_crs_csi2_swnodes(); > > + > > acpi_bus_attach(adev, NULL); > > } > > > > @@ -2516,11 +2523,12 @@ int acpi_bus_scan(acpi_handle handle) > > return -ENODEV; > > > > /* > > - * Allocate ACPI _CRS CSI-2 software nodes using information extracted > > + * Set up ACPI _CRS CSI-2 software nodes using information extracted > > * from the _CRS CSI-2 resource descriptors during the ACPI namespace > > - * walk above. > > + * walk above and MIPI DiSco for Imaging device properties. > > */ > > acpi_mipi_scan_crs_csi2(); > > + acpi_mipi_init_crs_csi2_swnodes(); > > > > acpi_bus_attach(device, (void *)true); > > > > Index: linux-pm/include/acpi/acpi_bus.h > > =================================================================== > > --- linux-pm.orig/include/acpi/acpi_bus.h > > +++ linux-pm/include/acpi/acpi_bus.h > > @@ -366,10 +366,17 @@ struct acpi_device_data { > > > > struct acpi_gpio_mapping; > > > > +#define ACPI_DEVICE_SWNODE_ROOT 0 > > + > > #define ACPI_DEVICE_CSI2_DATA_LANES 4 > > > > #define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH 8 > > > > +enum acpi_device_swnode_dev_props { > > + ACPI_DEVICE_SWNODE_DEV_NUM_OF, > > + ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES > > +}; > > + > > enum acpi_device_swnode_port_props { > > ACPI_DEVICE_SWNODE_PORT_REG, > > ACPI_DEVICE_SWNODE_PORT_NUM_OF, > > @@ -425,12 +432,14 @@ struct acpi_device_software_node_port { > > > > /** > > * struct acpi_device_software_nodes - Software nodes for an ACPI device > > + * @dev_props: Device properties. > > * @nodes: Software nodes for root as well as ports and endpoints. > > * @nodeprts: Array of software node pointers, for (un)registering them. > > * @ports: Information related to each port and endpoint within a port. > > * @num_ports: The number of ports. > > */ > > struct acpi_device_software_nodes { > > + struct property_entry dev_props[ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES]; > > struct software_node *nodes; > > const struct software_node **nodeptrs; > > struct acpi_device_software_node_port *ports; > > @@ -455,6 +464,7 @@ struct acpi_device { > > struct acpi_device_data data; > > struct acpi_scan_handler *handler; > > struct acpi_hotplug_context *hp; > > + struct acpi_device_software_nodes *swnodes; > > const struct acpi_gpio_mapping *driver_gpios; > > void *driver_data; > > struct device dev; > > > > > > > > -- > Regards, > > Sakari Ailus >
Hi Rafael, On Tue, Oct 31, 2023 at 02:46:51PM +0100, Rafael J. Wysocki wrote: > Hi Sakari, > > On Tue, Oct 31, 2023 at 11:33 AM Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: > > > > Hi Rafael, > > > > On subject: > > > > s/DiSco/DisCo/ > > Huh > > Thanks for catching this! > > > On Fri, Oct 20, 2023 at 04:39:27PM +0200, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > Add information extracted from the MIPI DiSco for Imaging device > > > > Ditto. > > > > > properties to software nodes created during the CSI-2 connection graph > > > discovery. > > > > > > Link: https://www.mipi.org/specifications/mipi-di > > > > This URL is broken. The correct URL is: > > > > https://www.mipi.org/specifications/mipi-disco-imaging > > OK > > > > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > --- > > > drivers/acpi/internal.h | 1 > > > drivers/acpi/mipi-di.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++- > > > > How about mipi-disco.c? I wouldn't mind having mipi-disco-imaging.c either. > > Well, mipi-disco-imaging is a bit too long and the meaning of > mipi-disco would not be clear IMO. > > What about mipi-disco-img? Seems fine to me. > > [cut] > > > > + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities"); > > > + if (ret > 0) { > > > + unsigned long mask; > > > + unsigned int i; > > > + > > > + /* > > > + * Total number of lanes here is clock lane + data lanes. > > > + * Require that number to be low enough so they all can be > > > + * covered by the bits in one byte. > > > + */ > > > + BUILD_BUG_ON(BITS_PER_TYPE(u8) <= ARRAY_SIZE(port->data_lanes)); > > > + > > > + fwnode_property_read_u8_array(port_fwnode, > > > + "mipi-img-lane-polarities", > > > + val, 1); > > > + > > > + for (mask = val[0], i = 0; i < num_lanes + 1; i++) > > > + port->lane_polarities[i] = test_bit(i, &mask); > > > > This works only up to seven lanes. > > True, but then ACPI_DEVICE_CSI2_DATA_LANES is defined as 4. The spec has no limit and in practice 4 is relatively common while 8 exists (but is somewhat rare). I actually think it'd be a good idea to bump this to 8 already.
Hi Sakari, On Tue, Oct 31, 2023 at 3:00 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Rafael, > > On Tue, Oct 31, 2023 at 02:46:51PM +0100, Rafael J. Wysocki wrote: > > Hi Sakari, > > > > On Tue, Oct 31, 2023 at 11:33 AM Sakari Ailus > > <sakari.ailus@linux.intel.com> wrote: > > > > > > Hi Rafael, > > > > > > On subject: > > > > > > s/DiSco/DisCo/ > > > > Huh > > > > Thanks for catching this! > > > > > On Fri, Oct 20, 2023 at 04:39:27PM +0200, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > > > Add information extracted from the MIPI DiSco for Imaging device > > > > > > Ditto. > > > > > > > properties to software nodes created during the CSI-2 connection graph > > > > discovery. > > > > > > > > Link: https://www.mipi.org/specifications/mipi-di > > > > > > This URL is broken. The correct URL is: > > > > > > https://www.mipi.org/specifications/mipi-disco-imaging > > > > OK > > > > > > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > --- > > > > drivers/acpi/internal.h | 1 > > > > drivers/acpi/mipi-di.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++- > > > > > > How about mipi-disco.c? I wouldn't mind having mipi-disco-imaging.c either. > > > > Well, mipi-disco-imaging is a bit too long and the meaning of > > mipi-disco would not be clear IMO. > > > > What about mipi-disco-img? > > Seems fine to me. > > > > > [cut] > > > > > > + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities"); > > > > + if (ret > 0) { > > > > + unsigned long mask; > > > > + unsigned int i; > > > > + > > > > + /* > > > > + * Total number of lanes here is clock lane + data lanes. > > > > + * Require that number to be low enough so they all can be > > > > + * covered by the bits in one byte. > > > > + */ > > > > + BUILD_BUG_ON(BITS_PER_TYPE(u8) <= ARRAY_SIZE(port->data_lanes)); > > > > + > > > > + fwnode_property_read_u8_array(port_fwnode, > > > > + "mipi-img-lane-polarities", > > > > + val, 1); > > > > + > > > > + for (mask = val[0], i = 0; i < num_lanes + 1; i++) > > > > + port->lane_polarities[i] = test_bit(i, &mask); > > > > > > This works only up to seven lanes. > > > > True, but then ACPI_DEVICE_CSI2_DATA_LANES is defined as 4. > > The spec has no limit and in practice 4 is relatively common while 8 exists > (but is somewhat rare). I actually think it'd be a good idea to bump this > to 8 already. OK
Index: linux-pm/drivers/acpi/mipi-di.c =================================================================== --- linux-pm.orig/drivers/acpi/mipi-di.c +++ linux-pm/drivers/acpi/mipi-di.c @@ -6,12 +6,16 @@ * * Support MIPI DisCo for Imaging by parsing ACPI _CRS CSI-2 records defined in * Section 6.4.3.8.2.4 "Camera Serial Interface (CSI-2) Connection Resource - * Descriptor" of ACPI 6.5. + * Descriptor" of ACPI 6.5 and using device properties defined by the MIPI DisCo + * for Imaging specification. * * The implementation looks for the information in the ACPI namespace (CSI-2 * resource descriptors in _CRS) and constructs software nodes compatible with * Documentation/firmware-guide/acpi/dsd/graph.rst to represent the CSI-2 - * connection graph. + * connection graph. The software nodes are then populated with the data + * extracted from the _CRS CSI-2 resource descriptors and the MIPI DisCo + * for Imaging device properties present in _DSD for the ACPI device objects + * with CSI-2 connections. */ #include <linux/acpi.h> @@ -434,6 +438,238 @@ void acpi_mipi_scan_crs_csi2(void) prepare_crs_csi2_swnodes(csi2); } +/* + * Get the index of the next property in the property array, with a given + * maximum value. + */ +#define NEXT_PROPERTY(index, max) \ + (WARN_ON((index) > ACPI_DEVICE_SWNODE_##max) ? \ + ACPI_DEVICE_SWNODE_##max : (index)++) + +static void init_csi2_port_local(struct acpi_device *adev, + struct acpi_device_software_node_port *port, + struct fwnode_handle *port_fwnode, + unsigned int index) +{ + acpi_handle handle = acpi_device_handle(adev); + unsigned int num_link_freqs; + int ret; + + ret = fwnode_property_count_u64(port_fwnode, "mipi-img-link-frequencies"); + if (ret <= 0) + return; + + num_link_freqs = ret; + if (num_link_freqs > ARRAY_SIZE(port->link_frequencies)) { + acpi_handle_info(handle, "Too many link frequencies: %u\n", + num_link_freqs); + num_link_freqs = ARRAY_SIZE(port->link_frequencies); + } + + ret = fwnode_property_read_u64_array(port_fwnode, + "mipi-img-link-frequencies", + port->link_frequencies, + num_link_freqs); + if (ret) { + acpi_handle_info(handle, "Unable to get link frequencies (%d)\n", + ret); + return; + } + + port->ep_props[NEXT_PROPERTY(index, EP_LINK_FREQUENCIES)] = + PROPERTY_ENTRY_U64_ARRAY_LEN("link-frequencies", + port->link_frequencies, + num_link_freqs); +} + +static void init_csi2_port(struct acpi_device *adev, + struct acpi_device_software_nodes *swnodes, + struct acpi_device_software_node_port *port, + struct fwnode_handle *port_fwnode, + unsigned int port_index) +{ + unsigned int ep_prop_index = ACPI_DEVICE_SWNODE_EP_CLOCK_LANES; + acpi_handle handle = acpi_device_handle(adev); + u8 val[ARRAY_SIZE(port->data_lanes)]; + unsigned int num_lanes = 0; + int ret; + + if (GRAPH_PORT_NAME(port->port_name, port->port_nr)) + return; + + swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)] = + SOFTWARE_NODE(port->port_name, port->port_props, + &swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT]); + + ret = fwnode_property_read_u8(port_fwnode, "mipi-img-clock-lane", val); + if (!ret) + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_CLOCK_LANES)] = + PROPERTY_ENTRY_U32("clock-lanes", val[0]); + + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-data-lanes"); + if (ret > 0) { + num_lanes = ret; + + if (num_lanes > ARRAY_SIZE(port->data_lanes)) { + acpi_handle_info(handle, "Too many data lanes: %u\n", + num_lanes); + num_lanes = ARRAY_SIZE(port->data_lanes); + } + + ret = fwnode_property_read_u8_array(port_fwnode, + "mipi-img-data-lanes", + val, num_lanes); + if (!ret) { + unsigned int i; + + for (i = 0; i < num_lanes; i++) + port->data_lanes[i] = val[i]; + + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_DATA_LANES)] = + PROPERTY_ENTRY_U32_ARRAY_LEN("data-lanes", + port->data_lanes, + num_lanes); + } + } + + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities"); + if (ret > 0) { + unsigned long mask; + unsigned int i; + + /* + * Total number of lanes here is clock lane + data lanes. + * Require that number to be low enough so they all can be + * covered by the bits in one byte. + */ + BUILD_BUG_ON(BITS_PER_TYPE(u8) <= ARRAY_SIZE(port->data_lanes)); + + fwnode_property_read_u8_array(port_fwnode, + "mipi-img-lane-polarities", + val, 1); + + for (mask = val[0], i = 0; i < num_lanes + 1; i++) + port->lane_polarities[i] = test_bit(i, &mask); + + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_LANE_POLARITIES)] = + PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities", + port->lane_polarities, + 1 + num_lanes); + } else { + acpi_handle_info(handle, "No lane polarity bytes\n"); + } + + swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] = + SOFTWARE_NODE("endpoint@0", swnodes->ports[port_index].ep_props, + &swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)]); + + if (port->crs_csi2_local) + init_csi2_port_local(adev, port, port_fwnode, ep_prop_index); +} + +#define MIPI_IMG_PORT_PREFIX "mipi-img-port-" + +static struct fwnode_handle *get_mipi_port_handle(struct fwnode_handle *adev_fwnode, + unsigned int port_nr) +{ + char port_name[sizeof(MIPI_IMG_PORT_PREFIX) + 2]; + + if (snprintf(port_name, sizeof(port_name), "%s%u", + MIPI_IMG_PORT_PREFIX, port_nr) >= sizeof(port_name)) + return NULL; + + return fwnode_get_named_child_node(adev_fwnode, port_name); +} + +static void init_crs_csi2_swnodes(struct crs_csi2 *csi2) +{ + struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER }; + struct acpi_device_software_nodes *swnodes = csi2->swnodes; + acpi_handle handle = csi2->handle; + struct fwnode_handle *adev_fwnode; + struct acpi_device *adev; + acpi_status status; + unsigned int i; + int ret; + + /* + * Bail out if the swnodes are not available (either they have not been + * allocated or they have been assigned to the device already). + */ + if (!swnodes) + return; + + adev = acpi_fetch_acpi_dev(handle); + if (!adev) + return; + + adev_fwnode = acpi_fwnode_handle(adev); + + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); + if (ACPI_FAILURE(status)) { + acpi_handle_info(handle, "Unable to get the path name\n"); + return; + } + + swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT] = + SOFTWARE_NODE(buffer.pointer, swnodes->dev_props, NULL); + + for (i = 0; i < swnodes->num_ports; i++) { + struct acpi_device_software_node_port *port = &swnodes->ports[i]; + struct fwnode_handle *port_fwnode; + + /* + * The MIPI DisCo for Imaging specification defines _DSD device + * properties for providing CSI-2 port parameters that can be + * accessed through the generic device properties framework. To + * access them, it is first necessary to find the data node + * representing the port under the given ACPI device object. + */ + port_fwnode = get_mipi_port_handle(adev_fwnode, port->port_nr); + if (!port_fwnode) { + acpi_handle_info(handle, + "MIPI port name too long for port %u\n", + port->port_nr); + continue; + } + + init_csi2_port(adev, swnodes, port, port_fwnode, i); + + fwnode_handle_put(port_fwnode); + } + + ret = software_node_register_node_group(swnodes->nodeptrs); + if (ret < 0) { + acpi_handle_info(handle, + "Unable to register software nodes (%d)\n", ret); + return; + } + + adev->swnodes = swnodes; + adev_fwnode->secondary = software_node_fwnode(swnodes->nodes); + + /* + * Prevents the swnodes from this csi2 entry from being assigned again + * or freed prematurely. + */ + csi2->swnodes = NULL; +} + +/** + * acpi_mipi_init_crs_csi2_swnodes - Initialize _CRS CSI-2 software nodes + * + * Use MIPI DiSco for Imaging device properties to finalize the initialization + * of CSI-2 software nodes for all ACPI device objects that have been already + * enumerated. + */ +void acpi_mipi_init_crs_csi2_swnodes(void) +{ + struct crs_csi2 *csi2, *csi2_tmp; + + list_for_each_entry_safe(csi2, csi2_tmp, &acpi_mipi_crs_csi2_list, entry) + init_crs_csi2_swnodes(csi2); +} + /** * acpi_mipi_crs_csi2_cleanup - Free _CRS CSI-2 temporary data */ Index: linux-pm/drivers/acpi/internal.h =================================================================== --- linux-pm.orig/drivers/acpi/internal.h +++ linux-pm/drivers/acpi/internal.h @@ -287,6 +287,7 @@ static inline void acpi_init_lpit(void) void acpi_mipi_check_crs_csi2(acpi_handle handle); void acpi_mipi_scan_crs_csi2(void); +void acpi_mipi_init_crs_csi2_swnodes(void); void acpi_mipi_crs_csi2_cleanup(void); #endif /* _ACPI_INTERNAL_H_ */ Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -2447,6 +2447,13 @@ static void acpi_scan_postponed_branch(a acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, acpi_bus_check_add_2, NULL, NULL, (void **)&adev); + + /* + * Populate the ACPI _CRS CSI-2 software nodes for the ACPI devices that + * have been added above. + */ + acpi_mipi_init_crs_csi2_swnodes(); + acpi_bus_attach(adev, NULL); } @@ -2516,11 +2523,12 @@ int acpi_bus_scan(acpi_handle handle) return -ENODEV; /* - * Allocate ACPI _CRS CSI-2 software nodes using information extracted + * Set up ACPI _CRS CSI-2 software nodes using information extracted * from the _CRS CSI-2 resource descriptors during the ACPI namespace - * walk above. + * walk above and MIPI DiSco for Imaging device properties. */ acpi_mipi_scan_crs_csi2(); + acpi_mipi_init_crs_csi2_swnodes(); acpi_bus_attach(device, (void *)true); Index: linux-pm/include/acpi/acpi_bus.h =================================================================== --- linux-pm.orig/include/acpi/acpi_bus.h +++ linux-pm/include/acpi/acpi_bus.h @@ -366,10 +366,17 @@ struct acpi_device_data { struct acpi_gpio_mapping; +#define ACPI_DEVICE_SWNODE_ROOT 0 + #define ACPI_DEVICE_CSI2_DATA_LANES 4 #define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH 8 +enum acpi_device_swnode_dev_props { + ACPI_DEVICE_SWNODE_DEV_NUM_OF, + ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES +}; + enum acpi_device_swnode_port_props { ACPI_DEVICE_SWNODE_PORT_REG, ACPI_DEVICE_SWNODE_PORT_NUM_OF, @@ -425,12 +432,14 @@ struct acpi_device_software_node_port { /** * struct acpi_device_software_nodes - Software nodes for an ACPI device + * @dev_props: Device properties. * @nodes: Software nodes for root as well as ports and endpoints. * @nodeprts: Array of software node pointers, for (un)registering them. * @ports: Information related to each port and endpoint within a port. * @num_ports: The number of ports. */ struct acpi_device_software_nodes { + struct property_entry dev_props[ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES]; struct software_node *nodes; const struct software_node **nodeptrs; struct acpi_device_software_node_port *ports; @@ -455,6 +464,7 @@ struct acpi_device { struct acpi_device_data data; struct acpi_scan_handler *handler; struct acpi_hotplug_context *hp; + struct acpi_device_software_nodes *swnodes; const struct acpi_gpio_mapping *driver_gpios; void *driver_data; struct device dev;