Re: [RFC PATCH 5/5] media: platform: Add Chrome OS EC CEC driver
On 05/15/2018 12:40 AM, Neil Armstrong wrote: > The Chrome OS Embedded Controller can expose a CEC bus, this patch add the > driver for such feature of the Embedded Controller. > > This driver is part of the cros-ec MFD and will be add as a sub-device when > the feature bit is exposed by the EC. > > The controller will only handle a single logical address and handles > all the messages retries and will only expose Success or Error. > > When the logical address is invalid, the controller will act as a CEC sniffer > and transfer all messages on the bus. I did not see any support for this. If this works as you state here, then adding support for CEC_CAP_MONITOR_ALL is highly recommended. > > Signed-off-by: Neil Armstrong > --- > drivers/media/platform/Kconfig | 11 + > drivers/media/platform/Makefile | 2 + > drivers/media/platform/cros-ec/Makefile | 1 + > drivers/media/platform/cros-ec/cros-ec-cec.c | 331 > +++ > 4 files changed, 345 insertions(+) > create mode 100644 drivers/media/platform/cros-ec/Makefile > create mode 100644 drivers/media/platform/cros-ec/cros-ec-cec.c Shouldn't the directory be called cros-ec-cec? > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index c7a1cf8..e55a8ed2 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -546,6 +546,17 @@ menuconfig CEC_PLATFORM_DRIVERS > > if CEC_PLATFORM_DRIVERS > > +config VIDEO_CROS_EC_CEC > + tristate "Chrome OS EC CEC driver" > + depends on MFD_CROS_EC || COMPILE_TEST > + select CEC_CORE > + select CEC_NOTIFIER > + ---help--- > + If you say yes here you will get support for the > + Chrome OS Embedded Controller's CEC. > + The CEC bus is present in the HDMI connector and enables communication > + between compatible devices. > + > config VIDEO_MESON_AO_CEC > tristate "Amlogic Meson AO CEC driver" > depends on ARCH_MESON || COMPILE_TEST > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile > index 932515d..0e0582e 100644 > --- a/drivers/media/platform/Makefile > +++ b/drivers/media/platform/Makefile > @@ -92,3 +92,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS) += > qcom/camss-8x16/ > obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/ > > obj-y+= meson/ > + > +obj-y+= cros-ec/ > diff --git a/drivers/media/platform/cros-ec/Makefile > b/drivers/media/platform/cros-ec/Makefile > new file mode 100644 > index 000..9ce97f9 > --- /dev/null > +++ b/drivers/media/platform/cros-ec/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_VIDEO_CROS_EC_CEC) += cros-ec-cec.o > diff --git a/drivers/media/platform/cros-ec/cros-ec-cec.c > b/drivers/media/platform/cros-ec/cros-ec-cec.c > new file mode 100644 > index 000..fea90da > --- /dev/null > +++ b/drivers/media/platform/cros-ec/cros-ec-cec.c > @@ -0,0 +1,331 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * CEC driver for Chrome OS Embedded Controller > + * > + * Copyright (c) 2018 BayLibre, SAS > + * Author: Neil Armstrong > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DRV_NAME "cros-ec-cec" > + > +/** > + * struct cros_ec_cec - Driver data for EC CEC > + * > + * @cros_ec: Pointer to EC device > + * @notifier: Notifier info for responding to EC events > + * @adap: CEC adapter > + * @notify: CEC notifier pointer > + * @rc_msg: storage for a received message > + */ > +struct cros_ec_cec { > + struct cros_ec_device *cros_ec; > + struct notifier_block notifier; > + struct cec_adapter *adap; > + struct cec_notifier *notify; > + struct cec_msg rx_msg; > +}; > + > +static void handle_cec_message(struct cros_ec_cec *cros_ec_cec) > +{ > + struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec; > + uint8_t *cec_message = cros_ec->event_data.data.cec_message; > + unsigned int len = cros_ec->event_size; > + > + cros_ec_cec->rx_msg.len = len; How robust is the underlying code and hardware? What happens if a CEC message with more than 16 bytes is received? Hard to test unless you have an RPi3 set up as a CEC debugger. See last section in https://hverkuil.home.xs4all.nl/cec-status.txt. Since you worked with CEC for a while now I recommend you set up such a system. It's cheap and very useful. > + memcpy(cros_ec_cec->rx_msg.msg, cec_message, len); > + > + cec_received_msg(cros_ec_cec->adap, &cros_ec_cec->rx_msg); > +} > + > +static void handle_cec_event(struct cros_ec_cec *cros_ec_cec) > +{ > + struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec; > + uint32_t events = cros_ec->event_data.data.cec_events; > + > + if (events & EC_MKBP_CEC_SEND_OK) > + cec_transmit_attempt_done(cros_ec_cec->adap, > +
Re: [RFC PATCH 3/5] drm/i915: hdmi: add CEC notifier to intel_hdmi
On 05/15/2018 12:40 AM, Neil Armstrong wrote: > This patchs adds the cec_notifier feature to the intel_hdmi part > of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate > between each HDMI ports. > The changes will allow the i915 HDMI code to notify EDID and HPD changes > to an eventual CEC adapter. > > Signed-off-by: Neil Armstrong > --- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_hdmi.c | 10 ++ The Kconfig also needs to be changed. In the DRM_I915 you need to add: select CEC_CORE if CEC_NOTIFIER Otherwise you'll get problems if the cec driver is a module and i915 is built-in. Regards, Hans
Re: [RFC PATCH 3/5] drm/i915: hdmi: add CEC notifier to intel_hdmi
On 05/15/2018 12:40 AM, Neil Armstrong wrote: > This patchs adds the cec_notifier feature to the intel_hdmi part > of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate > between each HDMI ports. > The changes will allow the i915 HDMI code to notify EDID and HPD changes > to an eventual CEC adapter. > > Signed-off-by: Neil Armstrong > --- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_hdmi.c | 10 ++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index d436858..b50e51b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -39,6 +39,7 @@ > #include > #include > #include > +#include > > /** > * __wait_for - magic wait macro > @@ -1001,6 +1002,7 @@ struct intel_hdmi { > bool has_audio; > bool rgb_quant_range_selectable; > struct intel_connector *attached_connector; > + struct cec_notifier *notifier; > }; > > struct intel_dp_mst_encoder; > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 1baef4a..9b94d72 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1868,6 +1868,8 @@ intel_hdmi_set_edid(struct drm_connector *connector) > connected = true; > } > > + cec_notifier_set_phys_addr_from_edid(intel_hdmi->notifier, edid); > + > return connected; > } > > @@ -1876,6 +1878,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool > force) > { > enum drm_connector_status status; > struct drm_i915_private *dev_priv = to_i915(connector->dev); > + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > connector->base.id, connector->name); > @@ -1891,6 +1894,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool > force) > > intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); > > + if (status != connector_status_connected) > + cec_notifier_phys_addr_invalidate(intel_hdmi->notifier); > + > return status; > } > > @@ -2358,6 +2364,10 @@ void intel_hdmi_init_connector(struct > intel_digital_port *intel_dig_port, > u32 temp = I915_READ(PEG_BAND_GAP_DATA); > I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd); > } > + > + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name); > + if (!intel_hdmi->notifier) > + DRM_DEBUG_KMS("CEC notifier get failed\n"); You 'get' the notifier here, but where is the cec_notifier_put when the connector is deleted? Regards, Hans > } > > void intel_hdmi_init(struct drm_i915_private *dev_priv, >
Re: [RFC PATCH 2/5] media: cec-notifier: Get notifier by device and connector name
Hi Neil, Thanks for this patch series! Some comments below: On 05/15/2018 12:40 AM, Neil Armstrong wrote: > In non device-tree world, we can need to get the notifier by the driver > name directly and eventually defer probe if not yet created. > > This patch adds a variant of the get function by using the device name > instead and will not create a notifier if not yet created. > > But the i915 driver exposes at least 2 HDMI connectors, this patch also > adds the possibility to add a connector name tied to the notifier device > to form a tuple and associate different CEC controllers for each HDMI > connectors. > > Signed-off-by: Neil Armstrong > --- > drivers/media/cec/cec-notifier.c | 30 --- > include/media/cec-notifier.h | 44 > ++-- > 2 files changed, 69 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/cec/cec-notifier.c > b/drivers/media/cec/cec-notifier.c > index 16dffa0..716070a 100644 > --- a/drivers/media/cec/cec-notifier.c > +++ b/drivers/media/cec/cec-notifier.c > @@ -21,6 +21,7 @@ struct cec_notifier { > struct list_head head; > struct kref kref; > struct device *dev; > + const char *conn; > struct cec_adapter *cec_adap; > void (*callback)(struct cec_adapter *adap, u16 pa); > > @@ -30,13 +31,34 @@ struct cec_notifier { > static LIST_HEAD(cec_notifiers); > static DEFINE_MUTEX(cec_notifiers_lock); > > -struct cec_notifier *cec_notifier_get(struct device *dev) > +struct cec_notifier *cec_notifier_get_byname(const char *name, > + const char *conn) > { > struct cec_notifier *n; > > mutex_lock(&cec_notifiers_lock); > list_for_each_entry(n, &cec_notifiers, head) { > - if (n->dev == dev) { > + if (!strcmp(dev_name(n->dev), name) && > + (!conn || !strcmp(n->conn, conn))) { > + kref_get(&n->kref); > + mutex_unlock(&cec_notifiers_lock); > + return n; > + } > + } > + mutex_unlock(&cec_notifiers_lock); > + > + return NULL; This doesn't seem right. For one it doesn't act like the other cec_notifier_get* functions in that it doesn't make a new notifier if it wasn't found yet in the list. For another, I think this function shouldn't be here at all. How about calling bus_find_device_by_name(), then use cec_notifier_get_conn()? > +} > +EXPORT_SYMBOL_GPL(cec_notifier_get_byname); > + > +struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char > *conn) > +{ > + struct cec_notifier *n; > + > + mutex_lock(&cec_notifiers_lock); > + list_for_each_entry(n, &cec_notifiers, head) { > + if (n->dev == dev && > + (!conn || !strcmp(n->conn, conn))) { > kref_get(&n->kref); > mutex_unlock(&cec_notifiers_lock); > return n; > @@ -46,6 +68,8 @@ struct cec_notifier *cec_notifier_get(struct device *dev) > if (!n) > goto unlock; > n->dev = dev; > + if (conn) > + n->conn = devm_kstrdup(dev, conn, GFP_KERNEL); The use of devm_kstrdup worries me. The problem is that when the 'dev' device is removed, this memory is also automatically freed. But the notifier might still have a reference through the CEC driver, so you end up with a n->conn pointer that points to freed memory. I think it is better to just use kstrdup and kfree it when the last notifier reference is released. > n->phys_addr = CEC_PHYS_ADDR_INVALID; > mutex_init(&n->lock); > kref_init(&n->kref); > @@ -54,7 +78,7 @@ struct cec_notifier *cec_notifier_get(struct device *dev) > mutex_unlock(&cec_notifiers_lock); > return n; > } > -EXPORT_SYMBOL_GPL(cec_notifier_get); > +EXPORT_SYMBOL_GPL(cec_notifier_get_conn); > > static void cec_notifier_release(struct kref *kref) > { > diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h > index cf0add7..70f2974 100644 > --- a/include/media/cec-notifier.h > +++ b/include/media/cec-notifier.h > @@ -20,6 +20,37 @@ struct cec_notifier; > #if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER) > > /** > + * cec_notifier_get_byname - find a cec_notifier for the given device name > + * and connector tuple. > + * @name: device name that sends the events. > + * @conn: the connector name from which the event occurs > + * > + * If a notifier for device @name exists, then increase the refcount and > + * return that notifier. > + * > + * If it doesn't exist, return NULL > + */ > +struct cec_notifier *cec_notifier_get_byname(const char *name, > + const char *conn); > + > +/** > + * cec_notifier_get_conn - find or create a new cec_notifier for the given > + * device and connector tuple. > + * @dev: device that sends the events. > + * @conn: the connector name from which t
Re: [PATCH v1 4/4] samples/bpf: an example of a raw IR decoder
On Mon, May 14, 2018 at 2:11 PM, Sean Young wrote: > This implements the grundig-16 IR protocol. > > Signed-off-by: Sean Young > --- > samples/bpf/Makefile | 4 + > samples/bpf/bpf_load.c| 9 +- > samples/bpf/grundig_decoder_kern.c| 112 ++ > samples/bpf/grundig_decoder_user.c| 54 +++ > tools/bpf/bpftool/prog.c | 1 + > tools/include/uapi/linux/bpf.h| 17 +++- > tools/testing/selftests/bpf/bpf_helpers.h | 6 ++ > 7 files changed, 200 insertions(+), 3 deletions(-) > create mode 100644 samples/bpf/grundig_decoder_kern.c > create mode 100644 samples/bpf/grundig_decoder_user.c > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 4d6a6edd4bf6..c6fa111f103a 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -44,6 +44,7 @@ hostprogs-y += xdp_monitor > hostprogs-y += xdp_rxq_info > hostprogs-y += syscall_tp > hostprogs-y += cpustat > +hostprogs-y += grundig_decoder > > # Libbpf dependencies > LIBBPF := ../../tools/lib/bpf/bpf.o ../../tools/lib/bpf/nlattr.o > @@ -95,6 +96,7 @@ xdp_monitor-objs := bpf_load.o $(LIBBPF) xdp_monitor_user.o > xdp_rxq_info-objs := bpf_load.o $(LIBBPF) xdp_rxq_info_user.o > syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o > cpustat-objs := bpf_load.o $(LIBBPF) cpustat_user.o > +grundig_decoder-objs := bpf_load.o $(LIBBPF) grundig_decoder_user.o > > # Tell kbuild to always build the programs > always := $(hostprogs-y) > @@ -148,6 +150,7 @@ always += xdp_rxq_info_kern.o > always += xdp2skb_meta_kern.o > always += syscall_tp_kern.o > always += cpustat_kern.o > +always += grundig_decoder_kern.o > > HOSTCFLAGS += -I$(objtree)/usr/include > HOSTCFLAGS += -I$(srctree)/tools/lib/ > @@ -193,6 +196,7 @@ HOSTLOADLIBES_xdp_monitor += -lelf > HOSTLOADLIBES_xdp_rxq_info += -lelf > HOSTLOADLIBES_syscall_tp += -lelf > HOSTLOADLIBES_cpustat += -lelf > +HOSTLOADLIBES_grundig_decoder += -lelf > > # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on > cmdline: > # make samples/bpf/ LLC=~/git/llvm/build/bin/llc > CLANG=~/git/llvm/build/bin/clang > diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c > index bebe4188b4b3..0fd389e95bb9 100644 > --- a/samples/bpf/bpf_load.c > +++ b/samples/bpf/bpf_load.c > @@ -69,6 +69,7 @@ static int load_and_attach(const char *event, struct > bpf_insn *prog, int size) > bool is_sockops = strncmp(event, "sockops", 7) == 0; > bool is_sk_skb = strncmp(event, "sk_skb", 6) == 0; > bool is_sk_msg = strncmp(event, "sk_msg", 6) == 0; > + bool is_ir_decoder = strncmp(event, "ir_decoder", 10) == 0; > size_t insns_cnt = size / sizeof(struct bpf_insn); > enum bpf_prog_type prog_type; > char buf[256]; > @@ -102,6 +103,8 @@ static int load_and_attach(const char *event, struct > bpf_insn *prog, int size) > prog_type = BPF_PROG_TYPE_SK_SKB; > } else if (is_sk_msg) { > prog_type = BPF_PROG_TYPE_SK_MSG; > + } else if (is_ir_decoder) { > + prog_type = BPF_PROG_TYPE_RAWIR_DECODER; > } else { > printf("Unknown event '%s'\n", event); > return -1; > @@ -116,7 +119,8 @@ static int load_and_attach(const char *event, struct > bpf_insn *prog, int size) > > prog_fd[prog_cnt++] = fd; > > - if (is_xdp || is_perf_event || is_cgroup_skb || is_cgroup_sk) > + if (is_xdp || is_perf_event || is_cgroup_skb || is_cgroup_sk || > + is_ir_decoder) > return 0; > > if (is_socket || is_sockops || is_sk_skb || is_sk_msg) { > @@ -607,7 +611,8 @@ static int do_load_bpf_file(const char *path, > fixup_map_cb fixup_map) > memcmp(shname, "cgroup/", 7) == 0 || > memcmp(shname, "sockops", 7) == 0 || > memcmp(shname, "sk_skb", 6) == 0 || > - memcmp(shname, "sk_msg", 6) == 0) { > + memcmp(shname, "sk_msg", 6) == 0 || > + memcmp(shname, "ir_decoder", 10) == 0) { > ret = load_and_attach(shname, data->d_buf, > data->d_size); > if (ret != 0) > diff --git a/samples/bpf/grundig_decoder_kern.c > b/samples/bpf/grundig_decoder_kern.c > new file mode 100644 > index ..c80f2c9cc69a > --- /dev/null > +++ b/samples/bpf/grundig_decoder_kern.c > @@ -0,0 +1,112 @@ > + > +#include > +#include > +#include "bpf_helpers.h" > +#include > + > +enum grundig_state { > + STATE_INACTIVE, > + STATE_HEADER_SPACE, > + STATE_LEADING_PULSE, > + STATE_BITS_SPACE, > + STATE_BITS_PULSE, > +}; > + > +struct decoder_state { > + u32 bits; > + enum grundig_state state; > + u32 count; > + u32 last_space; > +}; > + > +struct bpf_map_def SEC("maps") decoder_state_map
Re: [PATCH v1 3/4] media: rc bpf: move ir_raw_event to uapi
On Mon, May 14, 2018 at 2:11 PM, Sean Young wrote: > The context provided to a BPF_PROG_RAWIR_DECODER is a struct ir_raw_event; > ensure user space has a a definition. > > Signed-off-by: Sean Young > --- > include/media/rc-core.h| 19 +-- > include/uapi/linux/bpf_rcdev.h | 24 Patch #2 already referenced this file. So if Patches #1 and #2 applied, there will be a compilation error. Could you re-arrange your patchset so that after sequentially applying each patch, there is no compilation error? > 2 files changed, 25 insertions(+), 18 deletions(-) > create mode 100644 include/uapi/linux/bpf_rcdev.h > > diff --git a/include/media/rc-core.h b/include/media/rc-core.h > index 6742fd86ff65..5d31e31d8ade 100644 > --- a/include/media/rc-core.h > +++ b/include/media/rc-core.h > @@ -21,6 +21,7 @@ > #include > #include > #include > +#include > #include > > /** > @@ -299,24 +300,6 @@ void rc_keydown_notimeout(struct rc_dev *dev, enum > rc_proto protocol, > void rc_keyup(struct rc_dev *dev); > u32 rc_g_keycode_from_table(struct rc_dev *dev, u32 scancode); > > -/* > - * From rc-raw.c > - * The Raw interface is specific to InfraRed. It may be a good idea to > - * split it later into a separate header. > - */ > -struct ir_raw_event { > - union { > - u32 duration; > - u32 carrier; > - }; > - u8 duty_cycle; > - > - unsignedpulse:1; > - unsignedreset:1; > - unsignedtimeout:1; > - unsignedcarrier_report:1; > -}; > - > #define DEFINE_IR_RAW_EVENT(event) struct ir_raw_event event = {} > > static inline void init_ir_raw_event(struct ir_raw_event *ev) > diff --git a/include/uapi/linux/bpf_rcdev.h b/include/uapi/linux/bpf_rcdev.h > new file mode 100644 > index ..d8629ff2b960 > --- /dev/null > +++ b/include/uapi/linux/bpf_rcdev.h > @@ -0,0 +1,24 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* Copyright (c) 2018 Sean Young > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of version 2 of the GNU General Public > + * License as published by the Free Software Foundation. > + */ > +#ifndef _UAPI__LINUX_BPF_RCDEV_H__ > +#define _UAPI__LINUX_BPF_RCDEV_H__ > + > +struct ir_raw_event { > + union { > + __u32 duration; > + __u32 carrier; > + }; > + __u8duty_cycle; > + > + unsignedpulse:1; > + unsignedreset:1; > + unsignedtimeout:1; > + unsignedcarrier_report:1; > +}; > + > +#endif /* _UAPI__LINUX_BPF_RCDEV_H__ */ > -- > 2.17.0 >
Re: [PATCH v1 2/4] media: bpf: allow raw IR decoder bpf programs to be used
On Mon, May 14, 2018 at 2:10 PM, Sean Young wrote: > This implements attaching, detaching, querying and execution. The target > fd has to be the /dev/lircN device. > > Signed-off-by: Sean Young > --- > drivers/media/rc/ir-bpf-decoder.c | 191 ++ > drivers/media/rc/lirc_dev.c | 30 + > drivers/media/rc/rc-core-priv.h | 15 +++ > drivers/media/rc/rc-ir-raw.c | 5 + > include/uapi/linux/bpf.h | 1 + > kernel/bpf/syscall.c | 7 ++ > 6 files changed, 249 insertions(+) > > diff --git a/drivers/media/rc/ir-bpf-decoder.c > b/drivers/media/rc/ir-bpf-decoder.c > index aaa5e208b1a5..651590a14772 100644 > --- a/drivers/media/rc/ir-bpf-decoder.c > +++ b/drivers/media/rc/ir-bpf-decoder.c > @@ -91,3 +91,194 @@ const struct bpf_verifier_ops ir_decoder_verifier_ops = { > .get_func_proto = ir_decoder_func_proto, > .is_valid_access = ir_decoder_is_valid_access > }; > + > +#define BPF_MAX_PROGS 64 > + > +int rc_dev_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog, u32 flags) flags is not used in this function. > +{ > + struct ir_raw_event_ctrl *raw; > + struct bpf_prog_array __rcu *old_array; > + struct bpf_prog_array *new_array; > + int ret; > + > + if (rcdev->driver_type != RC_DRIVER_IR_RAW) > + return -EINVAL; > + > + ret = mutex_lock_interruptible(&rcdev->lock); > + if (ret) > + return ret; > + > + raw = rcdev->raw; > + > + if (raw->progs && bpf_prog_array_length(raw->progs) >= BPF_MAX_PROGS) > { > + ret = -E2BIG; > + goto out; > + } > + > + old_array = raw->progs; > + ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array); > + if (ret < 0) > + goto out; > + > + rcu_assign_pointer(raw->progs, new_array); > + bpf_prog_array_free(old_array); > +out: > + mutex_unlock(&rcdev->lock); > + return ret; > +} > + > +int rc_dev_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog, u32 flags) flags is not used in this function. > +{ > + struct ir_raw_event_ctrl *raw; > + struct bpf_prog_array __rcu *old_array; > + struct bpf_prog_array *new_array; > + int ret; > + > + if (rcdev->driver_type != RC_DRIVER_IR_RAW) > + return -EINVAL; > + > + ret = mutex_lock_interruptible(&rcdev->lock); > + if (ret) > + return ret; > + > + raw = rcdev->raw; > + > + old_array = raw->progs; > + ret = bpf_prog_array_copy(old_array, prog, NULL, &new_array); > + if (ret < 0) { > + bpf_prog_array_delete_safe(old_array, prog); > + } else { > + rcu_assign_pointer(raw->progs, new_array); > + bpf_prog_array_free(old_array); > + } > + > + bpf_prog_put(prog); > + mutex_unlock(&rcdev->lock); > + return 0; > +} > + > +void rc_dev_bpf_run(struct rc_dev *rcdev) > +{ > + struct ir_raw_event_ctrl *raw = rcdev->raw; > + > + if (raw->progs) > + BPF_PROG_RUN_ARRAY(raw->progs, &raw->prev_ev, BPF_PROG_RUN); > +} > + > +void rc_dev_bpf_put(struct rc_dev *rcdev) > +{ > + struct bpf_prog_array *progs = rcdev->raw->progs; > + int i, size; > + > + if (!progs) > + return; > + > + size = bpf_prog_array_length(progs); > + for (i = 0; i < size; i++) > + bpf_prog_put(progs->progs[i]); > + > + bpf_prog_array_free(rcdev->raw->progs); > +} > + > +int rc_dev_prog_attach(const union bpf_attr *attr) > +{ > + struct bpf_prog *prog; > + struct rc_dev *rcdev; > + int ret; > + > + if (attr->attach_flags & BPF_F_ALLOW_OVERRIDE) > + return -EINVAL; Looks like you really did not use flags except here. BPF_F_ALLOW_OVERRIDE is originally used for cgroup type of attachment and the comment explicits saying so. In the query below, the flags value "0" is copied to userspace. In your case, I think you can just disallow any value, i.g., attr->attach_flags must be 0, and then you further down check that if the prog is already in the array, you just return an error. > + > + prog = bpf_prog_get_type(attr->attach_bpf_fd, > +BPF_PROG_TYPE_RAWIR_DECODER); > + if (IS_ERR(prog)) > + return PTR_ERR(prog); > + > + rcdev = rc_dev_get_from_fd(attr->target_fd); > + if (IS_ERR(rcdev)) { > + bpf_prog_put(prog); > + return PTR_ERR(rcdev); > + } > + > + ret = rc_dev_bpf_attach(rcdev, prog, attr->attach_flags); > + if (ret) > + bpf_prog_put(prog); > + > + put_device(&rcdev->dev); > + > + return ret; > +} > + > +int rc_dev_prog_detach(const union bpf_attr *attr) > +{ > + struct bpf_prog *prog; > + struct rc_dev *rcdev; > + int ret; > + > + if (attr->attach_flags & BPF_F_ALLOW_OVERRIDE) >
Re: [PATCH v16 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2
Hi Niklas, On Tuesday, 15 May 2018 03:56:33 EEST Niklas Söderlund wrote: > Hi, > > This is the latest incarnation of R-Car MIPI CSI-2 receiver driver. It's > based on top of the media-tree and are tested on Renesas Salvator-X and > Salvator-XS together with adv7482 and the now in tree rcar-vin driver :-) > > I hope this is the last incarnation of this patch-set, I do think it is > ready for upstream consumption :-) So do I. Even though you dropped Hans' reviewed-by tag due to changes in the hardware initialization code, I think the part that Hans cares about the most is the V4L2 API implementation, so I believe his review still applies. In my opinion the series has received the necessary review. Hans, would you like to take this through your tree, or should we send a pull request directly to Mauro ? I'd like the two patches to be merged in v4.18 if possible. > * Changes since v15 > - Merge struct phtw_mbps and struct phypll_hsfreqrange into a new struct > which maps a mpbs value to a register value, struct rcsi2_mbps_reg. > - Reduced number of loops and delay when waiting for LP-11 and > confirmation of PHTW write as suggested by Laurent. > - Dropped dev_dbg() printouts of the requested link speed. > - Fix small issues in comments. > - Remove unneeded () in for-loop condition in rcsi2_phtw_write_array(). > - Remove __refdata from declaration of 'static struct platform_driver > rcar_csi2_pdrv'. > - Update MODULE_DESCRIPTION to 'Renesas R-Car MIPI CSI-2 receiver > driver'. > - Fixed two erroneous values in hsfreqrange_h3_v3h_m3n[]. Thanks Jacopo > for spotting this! > - Max link speed for V3M and E3 are 1.125Gbps remove settings above that > limit in phtw_mbps_v3m_e3. This also changed in datasheet v1.0. > - Add review tags from Laurent and Maxime. > > * Changes since v14. > - Data sheet update changed init sequence for PHY forcing a restructure > of the driver. The restructure was so big I felt compel to drop all > review tags :-( > - The change was that the Renesas H3 procedure was aligned with other > SoC in the Gen3 family procedure. I had kept the rework as separate > patches and was planing to post once original driver with H3 and M3-W > support where merged. As review tags are dropped I chosen to squash > those patches into 2/2. > - Add support for Gen3 M3-N. > - Add support for Gen3 V3M. > - Set PHTC_TESTCLR when stopping the PHY. > - Revert back to the v12 and earlier phypll calculation as it turns out > it was correct after all. > - Added compatible string for R8A77965 and R8A77970. > - s/Port 0/port@0/ > - s/Port 1/port@1/ > - s/Endpoint 0/endpoint@0/ > > * Changes since v13 > - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i]. > - Add define for PHCLM_STOPSTATECKL. > - Update spelling in comments. > - Update calculation in rcar_csi2_calc_phypll() according to > https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one > before v14 did not take into account that 2 bits per sample is > transmitted. > - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch > statement to set correct number of lanes to enable. > - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match > style of rest of file. > - Switch to %u instead of 0x%x when printing bus type. > - Switch to %u instead of %d for priv->lanes which is unsigned. > - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in > rcar_csi2_formats[]. > - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16. > - Set INTSTATE after PL-11 is confirmed to match flow chart in > datasheet. > - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs. > - Add Maxime's and laurent's tags. > > * Changes since v12 > - Fixed spelling mistakes in commit messages and documentation patch, > thanks Laurent. > - Mark endpoints in port 1 as optional as it is allowed to not connect a > VIN to the CSI-2 and instead have it only use its parallel input > source (for those VIN that have one). > - Added Ack from Sakari, thanks! > - Media bus codes are u32 not unsigned int. > - Ignore error check for v4l2_subdev_call(sd, video, s_stream, 0) > - Do not set subdev host data as it's not used, > v4l2_set_subdev_hostdata(). > - Fixed spelling errors in commit message. > - Add SPDX header > - Rename badly named variable tmp to vcdt_part. > - Cache subdevice in bound callback instead of looking it up in the > media graph. By doing this rcar_csi2_sd_info() can be removed. > - Print unsigned using %u not %d. > - Call pm_runtime_enable() before calling v4l2_async_register_subdev(). > - Dropped of_match_ptr() as OF is required. > > * Changes since v11 > - Added missing call to v4l2_async_notifier_unregister(). > - Fixed missing reg popery in bindings documentation. > - Add Rob's ack to 01/02. > - Dropped 'media:' prefix from patch subjects as it seems they are added > first when a patch is picked up by the maintainer. > - Fixed typo in comment enpo
Re: [PATCH v1 1/4] media: rc: introduce BPF_PROG_IR_DECODER
On Mon, May 14, 2018 at 2:10 PM, Sean Young wrote: > Add support for BPF_PROG_IR_DECODER. This type of BPF program can call > rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report > that the last key should be repeated. > > Signed-off-by: Sean Young > --- > drivers/media/rc/Kconfig | 8 +++ > drivers/media/rc/Makefile | 1 + > drivers/media/rc/ir-bpf-decoder.c | 93 +++ > include/linux/bpf_types.h | 3 + > include/uapi/linux/bpf.h | 16 +- > 5 files changed, 120 insertions(+), 1 deletion(-) > create mode 100644 drivers/media/rc/ir-bpf-decoder.c > > diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig > index eb2c3b6eca7f..10ad6167d87c 100644 > --- a/drivers/media/rc/Kconfig > +++ b/drivers/media/rc/Kconfig > @@ -120,6 +120,14 @@ config IR_IMON_DECODER >remote control and you would like to use it with a raw IR >receiver, or if you wish to use an encoder to transmit this IR. > > +config IR_BPF_DECODER > + bool "Enable IR raw decoder using BPF" > + depends on BPF_SYSCALL > + depends on RC_CORE=y > + help > + Enable this option to make it possible to load custom IR > + decoders written in BPF. > + > endif #RC_DECODERS > > menuconfig RC_DEVICES > diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile > index 2e1c87066f6c..12e1118430d0 100644 > --- a/drivers/media/rc/Makefile > +++ b/drivers/media/rc/Makefile > @@ -5,6 +5,7 @@ obj-y += keymaps/ > obj-$(CONFIG_RC_CORE) += rc-core.o > rc-core-y := rc-main.o rc-ir-raw.o > rc-core-$(CONFIG_LIRC) += lirc_dev.o > +rc-core-$(CONFIG_IR_BPF_DECODER) += ir-bpf-decoder.o > obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o > obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o > obj-$(CONFIG_IR_RC6_DECODER) += ir-rc6-decoder.o > diff --git a/drivers/media/rc/ir-bpf-decoder.c > b/drivers/media/rc/ir-bpf-decoder.c > new file mode 100644 > index ..aaa5e208b1a5 > --- /dev/null > +++ b/drivers/media/rc/ir-bpf-decoder.c > @@ -0,0 +1,93 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// ir-bpf-decoder.c - handles bpf decoders > +// > +// Copyright (C) 2018 Sean Young > + > +#include > +#include > +#include "rc-core-priv.h" > + > +/* > + * BPF interface for raw IR decoder > + */ > +const struct bpf_prog_ops ir_decoder_prog_ops = { > +}; > + > +BPF_CALL_1(bpf_rc_repeat, struct ir_raw_event*, event) > +{ > + struct ir_raw_event_ctrl *ctrl; > + > + ctrl = container_of(event, struct ir_raw_event_ctrl, prev_ev); > + > + rc_repeat(ctrl->dev); > + return 0; > +} > + > +static const struct bpf_func_proto rc_repeat_proto = { > + .func = bpf_rc_repeat, > + .gpl_only = true, // rc_repeat is EXPORT_SYMBOL_GPL > + .ret_type = RET_VOID, I suggest using RET_INTEGER here since we do return an integer 0. RET_INTEGER will also make it easy to extend if you want to return a non-zero value for error code or other reasons. > + .arg1_type = ARG_PTR_TO_CTX, > +}; > + > +BPF_CALL_4(bpf_rc_keydown, struct ir_raw_event*, event, u32, protocol, > + u32, scancode, u32, toggle) > +{ > + struct ir_raw_event_ctrl *ctrl; > + > + ctrl = container_of(event, struct ir_raw_event_ctrl, prev_ev); > + rc_keydown(ctrl->dev, protocol, scancode, toggle != 0); > + return 0; > +} > + > +static const struct bpf_func_proto rc_keydown_proto = { > + .func = bpf_rc_keydown, > + .gpl_only = true, // rc_keydown is EXPORT_SYMBOL_GPL > + .ret_type = RET_VOID, ditto. RET_INTEGER is preferable. > + .arg1_type = ARG_PTR_TO_CTX, > + .arg2_type = ARG_ANYTHING, > + .arg3_type = ARG_ANYTHING, > + .arg4_type = ARG_ANYTHING, > +}; > + > +static const struct bpf_func_proto *ir_decoder_func_proto(enum bpf_func_id > func_id, const struct bpf_prog *prog) > +{ > + switch (func_id) { > + case BPF_FUNC_rc_repeat: > + return &rc_repeat_proto; > + case BPF_FUNC_rc_keydown: > + return &rc_keydown_proto; > + case BPF_FUNC_map_lookup_elem: > + return &bpf_map_lookup_elem_proto; > + case BPF_FUNC_map_update_elem: > + return &bpf_map_update_elem_proto; > + case BPF_FUNC_map_delete_elem: > + return &bpf_map_delete_elem_proto; > + case BPF_FUNC_ktime_get_ns: > + return &bpf_ktime_get_ns_proto; > + case BPF_FUNC_tail_call: > + return &bpf_tail_call_proto; > + case BPF_FUNC_get_prandom_u32: > + return &bpf_get_prandom_u32_proto; > + default: > + return NULL; > + } > +} > + > +static bool ir_decoder_is_valid_access(int off, int size, > + enum bpf_access_type type, > + const struct bpf_prog *prog, > + struct bpf_insn_access_aux *info
Re: [PATCH] media: dvb-frontends: add Socionext SC1501A ISDB-S/T demodulator driver
Hi Katsuhiro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.17-rc5 next-20180514] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Katsuhiro-Suzuki/media-dvb-frontends-add-Socionext-SC1501A-ISDB-S-T-demodulator-driver/20180515-091453 base: git://linuxtv.org/media_tree.git master reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/media/dvb-frontends/sc1501a.c:313:47: sparse: constant 211243671486 >> is so big it is long vim +313 drivers/media/dvb-frontends/sc1501a.c 258 259 static int sc1501a_s_read_status(struct sc1501a_priv *chip, 260 struct dtv_frontend_properties *c, 261 enum fe_status *status) 262 { 263 struct regmap *r_s = chip->regmap_s; 264 u32 cpmon, tmpu, tmpl, flg; 265 u64 tmp; 266 267 /* Sync detection */ 268 regmap_read(r_s, CPMON1_S, &cpmon); 269 270 *status = 0; 271 if (cpmon & CPMON1_S_FSYNC) 272 *status |= FE_HAS_VITERBI | FE_HAS_SYNC | FE_HAS_LOCK; 273 if (cpmon & CPMON1_S_W2LOCK) 274 *status |= FE_HAS_SIGNAL | FE_HAS_CARRIER; 275 276 /* Signal strength */ 277 c->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE; 278 279 if (*status & FE_HAS_SIGNAL) { 280 u32 agc; 281 282 regmap_read(r_s, AGCREAD_S, &tmpu); 283 agc = tmpu << 8; 284 285 c->strength.len = 1; 286 c->strength.stat[0].scale = FE_SCALE_RELATIVE; 287 c->strength.stat[0].uvalue = agc; 288 } 289 290 /* C/N rate */ 291 c->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE; 292 293 if (*status & FE_HAS_VITERBI) { 294 u32 cnr = 0, x, y, d; 295 u64 d_3 = 0; 296 297 regmap_read(r_s, CNRDXU_S, &tmpu); 298 regmap_read(r_s, CNRDXL_S, &tmpl); 299 x = (tmpu << 8) | tmpl; 300 regmap_read(r_s, CNRDYU_S, &tmpu); 301 regmap_read(r_s, CNRDYL_S, &tmpl); 302 y = (tmpu << 8) | tmpl; 303 304 /* CNR[dB]: 10 * log10(D) - 30.74 / D^3 - 3 */ 305 /* D = x^2 / (2^15 * y - x^2) */ 306 d = (y << 15) - x * x; 307 if (d > 0) { 308 /* (2^4 * D)^3 = 2^12 * D^3 */ 309 /* 3.074 * 2^(12 + 24) = 211243671486 */ 310 d_3 = div_u64(16 * x * x, d); 311 d_3 = d_3 * d_3 * d_3; 312 if (d_3) > 313 d_3 = div_u64(211243671486, d_3); 314 } 315 316 if (d_3) { 317 /* 0.3 * 2^24 = 5033164 */ 318 tmp = (s64)2 * intlog10(x) - intlog10(abs(d)) - d_3 319 - 5033164; 320 cnr = div_u64(tmp * 1, 1 << 24); 321 } 322 323 if (cnr) { 324 c->cnr.len = 1; 325 c->cnr.stat[0].scale = FE_SCALE_DECIBEL; 326 c->cnr.stat[0].uvalue = cnr; 327 } 328 } 329 330 /* BER */ 331 c->post_bit_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE; 332 c->post_bit_count.stat[0].scale = FE_SCALE_NOT_AVAILABLE; 333 334 regmap_read(r_s, BERCNFLG_S, &flg); 335 336 if ((*status & FE_HAS_VITERBI) && (flg & BERCNFLG_S_BERVRDY)) { 337 u32 bit_err, bit_cnt; 338 339 regmap_read(r_s, BERVRDU_S, &tmpu); 340 regmap_read(r_s, BERVRDL_S, &tmpl); 341 bit_err = (tmpu << 8) | tmpl; 342 bit_cnt = (1 << 13) * 204; 343 344 if (bit_cnt) { 345 c->post_bit_error.len = 1; 346 c->post_bit_error.stat[0].scale = FE_SCALE_COUNTER; 347 c->post_bit_error.stat[0].uvalue = bit_err; 348 c->post_bit_count.len = 1; 349 c->pos
Re: [PATCH 01/11] media: tm6000: fix potential Spectre variant 1
Hi Mauro, On 04/26/2018 06:42 PM, Mauro Carvalho Chehab wrote: I noticed you changed the status of this series from rejected to new. Yes. Also, there are other similar issues in media/pci/ Well, the issues will be there everywhere on all media drivers. I marked your patches because I need to study it carefully, after Peter's explanations. My plan is to do it next week. Still not sure if the approach you took is the best one or not. As I said, one possibility is to change the way v4l2-core handles VIDIOC_ENUM_foo ioctls, but that would be make harder to -stable backports. I need a weekend to sleep on it. I'm curious about how you finally resolved to handle these issues. I noticed Smatch is no longer reporting them. Thanks -- Gustavo
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue May 15 05:00:12 CEST 2018 media-tree git hash:2a5f2705c97625aa1a4e1dd4d584eaa05392e060 media_build git hash: d72556c0502c096c089c99c58ee4a02a13133361 v4l-utils git hash: 51cfcefe65225430fd6794301adcaae133ebfc2d gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2-RC1 smatch version: 0.5.1 host hardware: x86_64 host os:4.15.0-3-amd64 linux-git-arm-at91: WARNINGS linux-git-arm-davinci: OK linux-git-arm-multi: WARNINGS linux-git-arm-pxa: WARNINGS linux-git-arm-stm32: OK linux-git-arm64: WARNINGS linux-git-i686: WARNINGS linux-git-mips: OK linux-git-powerpc64: WARNINGS linux-git-sh: OK linux-git-x86_64: WARNINGS Check COMPILE_TEST: OK linux-2.6.36.4-i686: WARNINGS linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.101-i686: WARNINGS linux-3.0.101-x86_64: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.101-i686: WARNINGS linux-3.2.101-x86_64: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.113-i686: WARNINGS linux-3.4.113-x86_64: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.10-i686: WARNINGS linux-3.7.10-x86_64: WARNINGS linux-3.8.13-i686: WARNINGS linux-3.8.13-x86_64: WARNINGS linux-3.9.11-i686: WARNINGS linux-3.9.11-x86_64: WARNINGS linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: WARNINGS linux-3.11.10-x86_64: WARNINGS linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: WARNINGS linux-3.13.11-x86_64: WARNINGS linux-3.14.79-i686: WARNINGS linux-3.14.79-x86_64: WARNINGS linux-3.15.10-i686: WARNINGS linux-3.15.10-x86_64: WARNINGS linux-3.16.56-i686: WARNINGS linux-3.16.56-x86_64: WARNINGS linux-3.17.8-i686: WARNINGS linux-3.17.8-x86_64: WARNINGS linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: WARNINGS linux-3.19.8-x86_64: WARNINGS linux-4.0.9-i686: WARNINGS linux-4.0.9-x86_64: WARNINGS linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: WARNINGS linux-4.2.8-x86_64: WARNINGS linux-4.3.6-i686: WARNINGS linux-4.3.6-x86_64: WARNINGS linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-i686: WARNINGS linux-4.6.7-x86_64: WARNINGS linux-4.7.10-i686: WARNINGS linux-4.7.10-x86_64: WARNINGS linux-4.8.17-i686: WARNINGS linux-4.8.17-x86_64: WARNINGS linux-4.9.91-i686: OK linux-4.9.91-x86_64: WARNINGS linux-4.10.17-i686: OK linux-4.10.17-x86_64: WARNINGS linux-4.11.12-i686: OK linux-4.11.12-x86_64: WARNINGS linux-4.12.14-i686: OK linux-4.12.14-x86_64: WARNINGS linux-4.13.16-i686: WARNINGS linux-4.13.16-x86_64: WARNINGS linux-4.14.31-i686: WARNINGS linux-4.14.31-x86_64: WARNINGS linux-4.15.14-i686: WARNINGS linux-4.15.14-x86_64: WARNINGS linux-4.16.8-i686: WARNINGS linux-4.16.8-x86_64: WARNINGS linux-4.17-rc4-i686: WARNINGS linux-4.17-rc4-x86_64: WARNINGS apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
RE: [PATCH v11] media: imx258: Add imx258 camera sensor driver
Hi Sakari, > -Original Message- > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > ow...@vger.kernel.org] On Behalf Of Sakari Ailus > Sent: Saturday, May 12, 2018 8:48 PM > To: Zheng, Jian Xu > Cc: Chen, JasonX Z ; Tomasz Figa > ; Yeh, Andy ; Linux Media > Mailing List ; Chiang, AlanX > ; Qiu, Tian Shu > Subject: Re: [PATCH v11] media: imx258: Add imx258 camera sensor driver > > On Thu, May 10, 2018 at 11:08:43AM +, Zheng, Jian Xu wrote: > > Hi Sakari & Jason, > > > > > -Original Message- > > > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > > > ow...@vger.kernel.org] On Behalf Of Sakari Ailus > > > Sent: Wednesday, May 9, 2018 5:43 PM > > > To: Chen, JasonX Z > > > Cc: Tomasz Figa ; Yeh, Andy > > > ; Linux Media Mailing List > > > ; Chiang, AlanX > > > > > > Subject: Re: [PATCH v11] media: imx258: Add imx258 camera sensor > > > driver > > > > > > Hi Jason, > > > > > > On Wed, May 09, 2018 at 09:28:30AM +, Chen, JasonX Z wrote: > > > > Hello Tomasz > > > > > > > > >> +/* Test Pattern Control */ > > > > >> +#define IMX258_REG_TEST_PATTERN0x0600 > > > > >> +#define IMX258_TEST_PATTERN_DISABLE0 > > > > >> +#define IMX258_TEST_PATTERN_SOLID_COLOR1 > > > > >> +#define IMX258_TEST_PATTERN_COLOR_BARS 2 #define > > > > >> +IMX258_TEST_PATTERN_GREY_COLOR 3 > > > > >> +#define IMX258_TEST_PATTERN_PN94 > > > > I suppose we only use IMX258_TEST_PATTERN_COLOR_BARS. I heard that > > we'd better remove the functions/code no one would use. Is that true? e.g. > > remove all h_flip and v_flip ioctls because it's not used by anyone. > > HFLIP and VFLIP support were AFAIR removed as they were not supported > correctly by the driver --- they do affect the pixel order, i.e. the format. So you mean that if the sensor is RAW format, we should not implement HFLIP/VFLIP. Because it changes the pixel order? I don't quite follow the logic here. In the meantime, if the logic works here, we need some other kind of HFLIP/VFLIP ioctl because it's supported by HW. > I see no reason to remove support for the additional test patterns if the > implementation is correct --- in this case, just a single integer value. Best Regards, Jianxu(Clive) Zheng
Re: [PATCH v1 2/4] media: bpf: allow raw IR decoder bpf programs to be used
Hi Sean, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.17-rc5] [cannot apply to next-20180514] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sean-Young/media-rc-introduce-BPF_PROG_IR_DECODER/20180515-093234 config: i386-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> drivers/media/rc/lirc_dev.c:32:10: fatal error: linux/bpf-rcdev.h: No such >> file or directory #include ^~~ compilation terminated. -- >> kernel/bpf/syscall.c:30:10: fatal error: linux/bpf-rcdev.h: No such file or >> directory #include ^~~ compilation terminated. vim +32 drivers/media/rc/lirc_dev.c 31 > 32 #include 33 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH] media: helene: add I2C device probe function
This patch adds I2C probe function to use dvb_module_probe() with this driver. Signed-off-by: Katsuhiro Suzuki --- drivers/media/dvb-frontends/helene.c | 88 ++-- drivers/media/dvb-frontends/helene.h | 2 + 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c index a0d0b53c91d7..04033f0c278b 100644 --- a/drivers/media/dvb-frontends/helene.c +++ b/drivers/media/dvb-frontends/helene.c @@ -666,7 +666,7 @@ static int helene_set_params_s(struct dvb_frontend *fe) return 0; } -static int helene_set_params(struct dvb_frontend *fe) +static int helene_set_params_t(struct dvb_frontend *fe) { u8 data[MAX_WRITE_REGSIZE]; u32 frequency; @@ -835,6 +835,19 @@ static int helene_set_params(struct dvb_frontend *fe) return 0; } +static int helene_set_params(struct dvb_frontend *fe) +{ + struct dtv_frontend_properties *p = &fe->dtv_property_cache; + + if (p->delivery_system == SYS_DVBT || + p->delivery_system == SYS_DVBT2 || + p->delivery_system == SYS_ISDBT || + p->delivery_system == SYS_DVBC_ANNEX_A) + return helene_set_params_t(fe); + + return helene_set_params_s(fe); +} + static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency) { struct helene_priv *priv = fe->tuner_priv; @@ -843,7 +856,7 @@ static int helene_get_frequency(struct dvb_frontend *fe, u32 *frequency) return 0; } -static const struct dvb_tuner_ops helene_tuner_ops = { +static const struct dvb_tuner_ops helene_tuner_ops_t = { .info = { .name = "Sony HELENE Ter tuner", .frequency_min = 100, @@ -853,7 +866,7 @@ static const struct dvb_tuner_ops helene_tuner_ops = { .init = helene_init, .release = helene_release, .sleep = helene_sleep, - .set_params = helene_set_params, + .set_params = helene_set_params_t, .get_frequency = helene_get_frequency, }; @@ -871,6 +884,20 @@ static const struct dvb_tuner_ops helene_tuner_ops_s = { .get_frequency = helene_get_frequency, }; +static const struct dvb_tuner_ops helene_tuner_ops = { + .info = { + .name = "Sony HELENE Sat/Ter tuner", + .frequency_min = 50, + .frequency_max = 12, + .frequency_step = 1000, + }, + .init = helene_init, + .release = helene_release, + .sleep = helene_sleep, + .set_params = helene_set_params, + .get_frequency = helene_get_frequency, +}; + /* power-on tuner * call once after reset */ @@ -1032,7 +1059,7 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe, if (fe->ops.i2c_gate_ctrl) fe->ops.i2c_gate_ctrl(fe, 0); - memcpy(&fe->ops.tuner_ops, &helene_tuner_ops, + memcpy(&fe->ops.tuner_ops, &helene_tuner_ops_t, sizeof(struct dvb_tuner_ops)); fe->tuner_priv = priv; dev_info(&priv->i2c->dev, @@ -1042,6 +1069,59 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe, } EXPORT_SYMBOL(helene_attach); +static int helene_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct helene_config *config = client->dev.platform_data; + struct dvb_frontend *fe = config->fe; + struct device *dev = &client->dev; + struct helene_priv *priv; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->i2c_address = client->addr; + priv->i2c = client->adapter; + priv->set_tuner_data = config->set_tuner_priv; + priv->set_tuner = config->set_tuner_callback; + priv->xtal = config->xtal; + + if (fe->ops.i2c_gate_ctrl) + fe->ops.i2c_gate_ctrl(fe, 1); + + if (helene_x_pon(priv) != 0) + return -EINVAL; + + if (fe->ops.i2c_gate_ctrl) + fe->ops.i2c_gate_ctrl(fe, 0); + + memcpy(&fe->ops.tuner_ops, &helene_tuner_ops, + sizeof(struct dvb_tuner_ops)); + fe->tuner_priv = priv; + i2c_set_clientdata(client, priv); + + dev_info(dev, "Sony HELENE attached on addr=%x at I2C adapter %p\n", +priv->i2c_address, priv->i2c); + + return 0; +} + +static const struct i2c_device_id helene_id[] = { + { "helene", }, + {} +}; +MODULE_DEVICE_TABLE(i2c, helene_id); + +static struct i2c_driver helene_driver = { + .driver = { + .name = "helene", + }, + .probe= helene_probe, + .id_table = helene_id, +}; +module_i2c_driver(helene_driver); + MODULE_DESCRIPTION("Sony HELENE Sat/Ter tuner driver"); MODULE_AUTHOR("Abylay Ospan "); MODULE_LICENSE("GPL"); diff --git a/drivers/media/dvb-frontends/helene.h b/drivers/media/dvb-frontends/helene.h index c9fc81c7e4e7..ceaa283708cb 1
Re: [PATCH v1 3/4] media: rc bpf: move ir_raw_event to uapi
Hi Sean, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v4.17-rc5] [cannot apply to next-20180514] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sean-Young/media-rc-introduce-BPF_PROG_IR_DECODER/20180515-093234 config: i386-tinyconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): >> ./usr/include/linux/bpf_rcdev.h:13: found __[us]{8,16,32,64} type without >> #include --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH v16 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are connected between the video sources and the video grabbers (VIN). Driver is based on a prototype by Koji Matsuoka in the Renesas BSP. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart Reviewed-by: Maxime Ripard --- * Changes since v15 - Merge struct phtw_mbps and struct phypll_hsfreqrange into a new struct which maps a mpbs value to a register value, struct rcsi2_mbps_reg. - Reduced number of loops and delay when waiting for LP-11 and confirmation of PHTW write as suggested by Laurent. - Dropped dev_dbg() printouts of the requested link speed. - Fix small issues in comments. - Remove unneeded () in for-loop condition in rcsi2_phtw_write_array(). - Remove __refdata from declaration of 'static struct platform_driver rcar_csi2_pdrv'. - Update MODULE_DESCRIPTION to 'Renesas R-Car MIPI CSI-2 receiver driver'. - Fixed two erroneous values in hsfreqrange_h3_v3h_m3n[]. Thanks Jacopo for spotting this! - Max link speed for V3M and E3 are 1.125Gbps remove settings above that limit in phtw_mbps_v3m_e3. This also changed in datasheet v1.0. - Add review tags from Laurent and Maxime. * Changes since v14 - Data sheet update changed init sequence for PHY forcing a restructure of the driver. The restructure was so big I felt compel to drop all review tags :-( - The change was that the Renesas H3 procedure was aligned with other SoC in the Gen3 family procedure. I had kept the rework as separate patches and was planing to post once original driver with H3 and M3-W support where merged. As review tags are dropped I chosen to squash those patches into 2/2. - Add support for Gen3 V3M. - Add support for Gen3 M3-N. - Set PHTC_TESTCLR when stopping the PHY. - Revert back to the v12 and earlier phypll calculation as it turns out it was correct after all. * Changes since v13 - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i]. - Add define for PHCLM_STOPSTATECKL. - Update spelling in comments. - Update calculation in rcar_csi2_calc_phypll() according to https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one before v14 did not take into account that 2 bits per sample is transmitted. - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch statement to set correct number of lanes to enable. - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match style of rest of file. - Switch to %u instead of 0x%x when printing bus type. - Switch to %u instead of %d for priv->lanes which is unsigned. - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in rcar_csi2_formats[]. - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16. - Set INTSTATE after PL-11 is confirmed to match flow chart in datasheet. - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs. - Add Maxime's and laurent's tags. --- drivers/media/platform/rcar-vin/Kconfig | 12 + drivers/media/platform/rcar-vin/Makefile|1 + drivers/media/platform/rcar-vin/rcar-csi2.c | 1084 +++ 3 files changed, 1097 insertions(+) create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig index 8fa7ee468c63afb9..d5835da6d4100d87 100644 --- a/drivers/media/platform/rcar-vin/Kconfig +++ b/drivers/media/platform/rcar-vin/Kconfig @@ -1,3 +1,15 @@ +config VIDEO_RCAR_CSI2 + tristate "R-Car MIPI CSI-2 Receiver" + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF + depends on ARCH_RENESAS || COMPILE_TEST + select V4L2_FWNODE + help + Support for Renesas R-Car MIPI CSI-2 receiver. + Supports R-Car Gen3 SoCs. + + To compile this driver as a module, choose M here: the + module will be called rcar-csi2. + config VIDEO_RCAR_VIN tristate "R-Car Video Input (VIN) Driver" depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && MEDIA_CONTROLLER diff --git a/drivers/media/platform/rcar-vin/Makefile b/drivers/media/platform/rcar-vin/Makefile index 48c5632c21dc060b..5ab803d3e7c1aa57 100644 --- a/drivers/media/platform/rcar-vin/Makefile +++ b/drivers/media/platform/rcar-vin/Makefile @@ -1,3 +1,4 @@ rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o +obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c new file mode 100644 index ..e64f07fe184e7675 --- /dev/null +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c @@ -0,0 +1,1084 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for Renesas R-Car MIPI CSI-2 Receiver + * + * Copyright (C) 2018 Renesas Electronics Corp. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#inc
[PATCH v16 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers are located between the video sources (CSI-2 transmitters) and the video grabbers (VIN) on Gen3 of Renesas R-Car SoC. Each CSI-2 device is connected to more than one VIN device which simultaneously can receive video from the same CSI-2 device. Each VIN device can also be connected to more than one CSI-2 device. The routing of which links are used is controlled by the VIN devices. There are only a few possible routes which are set by hardware limitations, which are different for each SoC in the Gen3 family. Signed-off-by: Niklas Söderlund Acked-by: Rob Herring Acked-by: Sakari Ailus Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- * Changes since v14. - Added compatible string for R8A77965 and R8A77970. - s/Port 0/port@0/ - s/Port 1/port@1/ - s/Endpoint 0/endpoint@0/ * Changes since v13 - Add Laurent's tag. --- .../bindings/media/renesas,rcar-csi2.txt | 101 ++ MAINTAINERS | 1 + 2 files changed, 102 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt new file mode 100644 index ..2d385b65b275bc58 --- /dev/null +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt @@ -0,0 +1,101 @@ +Renesas R-Car MIPI CSI-2 + + +The R-Car CSI-2 receiver device provides MIPI CSI-2 capabilities for the +Renesas R-Car family of devices. It is used in conjunction with the +R-Car VIN module, which provides the video capture capabilities. + +Mandatory properties + + - compatible: Must be one or more of the following + - "renesas,r8a7795-csi2" for the R8A7795 device. + - "renesas,r8a7796-csi2" for the R8A7796 device. + - "renesas,r8a77965-csi2" for the R8A77965 device. + - "renesas,r8a77970-csi2" for the R8A77970 device. + + - reg: the register base and size for the device registers + - interrupts: the interrupt for the device + - clocks: reference to the parent clock + +The device node shall contain two 'port' child nodes according to the +bindings defined in Documentation/devicetree/bindings/media/ +video-interfaces.txt. port@0 shall connect to the CSI-2 source. port@1 +shall connect to all the R-Car VIN modules that have a hardware +connection to the CSI-2 receiver. + +- port@0- Video source (mandatory) + - endpoint@0 - sub-node describing the endpoint that is the video source + +- port@1 - VIN instances (optional) + - One endpoint sub-node for every R-Car VIN instance which is connected + to the R-Car CSI-2 receiver. + +Example: + + csi20: csi2@fea8 { + compatible = "renesas,r8a7796-csi2"; + reg = <0 0xfea8 0 0x1>; + interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 714>; + power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; + resets = <&cpg 714>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + #address-cells = <1>; + #size-cells = <0>; + + reg = <0>; + + csi20_in: endpoint@0 { + reg = <0>; + clock-lanes = <0>; + data-lanes = <1>; + remote-endpoint = <&adv7482_txb>; + }; + }; + + port@1 { + #address-cells = <1>; + #size-cells = <0>; + + reg = <1>; + + csi20vin0: endpoint@0 { + reg = <0>; + remote-endpoint = <&vin0csi20>; + }; + csi20vin1: endpoint@1 { + reg = <1>; + remote-endpoint = <&vin1csi20>; + }; + csi20vin2: endpoint@2 { + reg = <2>; + remote-endpoint = <&vin2csi20>; + }; + csi20vin3: endpoint@3 { + reg = <3>; + remote-endpoint = <&vin3csi20>; + }; + csi20vin4: endpoint@4 { + reg = <4>; + remote-endpoint = <&vin
[PATCH v16 0/2] rcar-csi2: add Renesas R-Car MIPI CSI-2
Hi, This is the latest incarnation of R-Car MIPI CSI-2 receiver driver. It's based on top of the media-tree and are tested on Renesas Salvator-X and Salvator-XS together with adv7482 and the now in tree rcar-vin driver :-) I hope this is the last incarnation of this patch-set, I do think it is ready for upstream consumption :-) * Changes since v15 - Merge struct phtw_mbps and struct phypll_hsfreqrange into a new struct which maps a mpbs value to a register value, struct rcsi2_mbps_reg. - Reduced number of loops and delay when waiting for LP-11 and confirmation of PHTW write as suggested by Laurent. - Dropped dev_dbg() printouts of the requested link speed. - Fix small issues in comments. - Remove unneeded () in for-loop condition in rcsi2_phtw_write_array(). - Remove __refdata from declaration of 'static struct platform_driver rcar_csi2_pdrv'. - Update MODULE_DESCRIPTION to 'Renesas R-Car MIPI CSI-2 receiver driver'. - Fixed two erroneous values in hsfreqrange_h3_v3h_m3n[]. Thanks Jacopo for spotting this! - Max link speed for V3M and E3 are 1.125Gbps remove settings above that limit in phtw_mbps_v3m_e3. This also changed in datasheet v1.0. - Add review tags from Laurent and Maxime. * Changes since v14. - Data sheet update changed init sequence for PHY forcing a restructure of the driver. The restructure was so big I felt compel to drop all review tags :-( - The change was that the Renesas H3 procedure was aligned with other SoC in the Gen3 family procedure. I had kept the rework as separate patches and was planing to post once original driver with H3 and M3-W support where merged. As review tags are dropped I chosen to squash those patches into 2/2. - Add support for Gen3 M3-N. - Add support for Gen3 V3M. - Set PHTC_TESTCLR when stopping the PHY. - Revert back to the v12 and earlier phypll calculation as it turns out it was correct after all. - Added compatible string for R8A77965 and R8A77970. - s/Port 0/port@0/ - s/Port 1/port@1/ - s/Endpoint 0/endpoint@0/ * Changes since v13 - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i]. - Add define for PHCLM_STOPSTATECKL. - Update spelling in comments. - Update calculation in rcar_csi2_calc_phypll() according to https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one before v14 did not take into account that 2 bits per sample is transmitted. - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch statement to set correct number of lanes to enable. - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match style of rest of file. - Switch to %u instead of 0x%x when printing bus type. - Switch to %u instead of %d for priv->lanes which is unsigned. - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in rcar_csi2_formats[]. - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16. - Set INTSTATE after PL-11 is confirmed to match flow chart in datasheet. - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs. - Add Maxime's and laurent's tags. * Changes since v12 - Fixed spelling mistakes in commit messages and documentation patch, thanks Laurent. - Mark endpoints in port 1 as optional as it is allowed to not connect a VIN to the CSI-2 and instead have it only use its parallel input source (for those VIN that have one). - Added Ack from Sakari, thanks! - Media bus codes are u32 not unsigned int. - Ignore error check for v4l2_subdev_call(sd, video, s_stream, 0) - Do not set subdev host data as it's not used, v4l2_set_subdev_hostdata(). - Fixed spelling errors in commit message. - Add SPDX header - Rename badly named variable tmp to vcdt_part. - Cache subdevice in bound callback instead of looking it up in the media graph. By doing this rcar_csi2_sd_info() can be removed. - Print unsigned using %u not %d. - Call pm_runtime_enable() before calling v4l2_async_register_subdev(). - Dropped of_match_ptr() as OF is required. * Changes since v11 - Added missing call to v4l2_async_notifier_unregister(). - Fixed missing reg popery in bindings documentation. - Add Rob's ack to 01/02. - Dropped 'media:' prefix from patch subjects as it seems they are added first when a patch is picked up by the maintainer. - Fixed typo in comment enpoint -> endpoint, thanks Hans. - Added Hans Reviewed-by to driver. * Changes since v10 - Renamed Documentation/devicetree/bindings/media/rcar-csi2.txt to Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt - Add extra newline in rcar_csi2_code_to_fmt() - Use locally stored format information instead of reading it from the remote subdevice, Sakari pointed out that the pipeline is validated before .s_stream() is called so this is safe. - Do not check return from media_entity_to_v4l2_subdev() in rcar_csi2_start(), Sakari pointed out it can't fail. Also move logic to find the remote subdevice is moved to the only user of it, rcar_csi2_calc_phypll(). - Move pm_runtime_get_sync() and pm_run
[PATCH] media: dvb-frontends: add Socionext SC1501A ISDB-S/T demodulator driver
This patch adds a frontend driver for the Socionext SC1501A series and Socionext MN88443x ISDB-S/T demodulators. Signed-off-by: Katsuhiro Suzuki --- drivers/media/dvb-frontends/Kconfig | 10 + drivers/media/dvb-frontends/Makefile | 1 + drivers/media/dvb-frontends/sc1501a.c | 802 ++ drivers/media/dvb-frontends/sc1501a.h | 27 + 4 files changed, 840 insertions(+) create mode 100644 drivers/media/dvb-frontends/sc1501a.c create mode 100644 drivers/media/dvb-frontends/sc1501a.h diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig index 55e36a4f5215..e9d2c94b290e 100644 --- a/drivers/media/dvb-frontends/Kconfig +++ b/drivers/media/dvb-frontends/Kconfig @@ -739,6 +739,16 @@ config DVB_TC90522 Toshiba TC90522 2xISDB-S 8PSK + 2xISDB-T OFDM demodulator. Say Y when you want to support this frontend. +config DVB_SC1501A + tristate "Socionext SC1501A" + depends on DVB_CORE && I2C + select REGMAP_I2C + default m if !MEDIA_SUBDRV_AUTOSELECT + help + A driver for Socionext SC1501A, MN884433 and MN884434 + ISDB-S + ISDB-T demodulator. + Say Y when you want to support this frontend. + comment "Digital terrestrial only tuners/PLL" depends on DVB_CORE diff --git a/drivers/media/dvb-frontends/Makefile b/drivers/media/dvb-frontends/Makefile index 67a783fd5ed0..e204502347ed 100644 --- a/drivers/media/dvb-frontends/Makefile +++ b/drivers/media/dvb-frontends/Makefile @@ -125,6 +125,7 @@ obj-$(CONFIG_DVB_AF9033) += af9033.o obj-$(CONFIG_DVB_AS102_FE) += as102_fe.o obj-$(CONFIG_DVB_GP8PSK_FE) += gp8psk-fe.o obj-$(CONFIG_DVB_TC90522) += tc90522.o +obj-$(CONFIG_DVB_SC1501A) += sc1501a.o obj-$(CONFIG_DVB_HORUS3A) += horus3a.o obj-$(CONFIG_DVB_ASCOT2E) += ascot2e.o obj-$(CONFIG_DVB_HELENE) += helene.o diff --git a/drivers/media/dvb-frontends/sc1501a.c b/drivers/media/dvb-frontends/sc1501a.c new file mode 100644 index ..6460d7f95b35 --- /dev/null +++ b/drivers/media/dvb-frontends/sc1501a.c @@ -0,0 +1,802 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Socionext SC1501A series demodulator driver for ISDB-S/ISDB-T. +// +// Copyright (c) 2018 Socionext Inc. + +#include +#include +#include +#include +#include +#include +#include + +#include "sc1501a.h" + +/* ISDB-S registers */ +#define ATSIDU_S0x2f +#define ATSIDL_S0x30 +#define TSSET_S 0x31 +#define AGCREAD_S 0x5a +#define CPMON1_S0x5e +#define CPMON1_S_FSYNC BIT(5) +#define CPMON1_S_ERRMON BIT(4) +#define CPMON1_S_SIGOFF BIT(3) +#define CPMON1_S_W2LOCK BIT(2) +#define CPMON1_S_W1LOCK BIT(1) +#define CPMON1_S_DW1LOCKBIT(0) +#define TRMON_S 0x60 +#define BERCNFLG_S 0x68 +#define BERCNFLG_S_BERVRDY BIT(5) +#define BERCNFLG_S_BERVCHK BIT(4) +#define BERCNFLG_S_BERDRDY BIT(3) +#define BERCNFLG_S_BERDCHK BIT(2) +#define CNRDXU_S0x69 +#define CNRDXL_S0x6a +#define CNRDYU_S0x6b +#define CNRDYL_S0x6c +#define BERVRDU_S 0x71 +#define BERVRDL_S 0x72 +#define DOSET1_S0x73 + +/* Primary ISDB-T */ +#define PLLASET10x00 +#define PLLASET20x01 +#define PLLBSET10x02 +#define PLLBSET20x03 +#define PLLSET 0x04 +#define OUTCSET 0x08 +#define OUTCSET_CHDRV_8MA 0xff +#define OUTCSET_CHDRV_4MA 0x00 +#define PLDWSET 0x09 +#define PLDWSET_NORMAL 0x00 +#define PLDWSET_PULLDOWN 0xff +#define HIZSET1 0x0a +#define HIZSET2 0x0b + +/* Secondary ISDB-T (for MN884434 only) */ +#define RCVSET 0x00 +#define TSSET1_M0x01 +#define TSSET2_M0x02 +#define TSSET3_M0x03 +#define INTACSET0x08 +#define HIZSET3 0x0b + +/* ISDB-T registe
Re: [PATCH v1 1/4] media: rc: introduce BPF_PROG_IR_DECODER
On 05/14/2018 02:10 PM, Sean Young wrote: > Add support for BPF_PROG_IR_DECODER. This type of BPF program can call Kconfig file below uses IR_BPF_DECODER instead of the symbol name above. and then patch 3 says a third choice: The context provided to a BPF_PROG_RAWIR_DECODER is a struct ir_raw_event; > rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report > that the last key should be repeated. > > Signed-off-by: Sean Young > --- > drivers/media/rc/Kconfig | 8 +++ > drivers/media/rc/Makefile | 1 + > drivers/media/rc/ir-bpf-decoder.c | 93 +++ > include/linux/bpf_types.h | 3 + > include/uapi/linux/bpf.h | 16 +- > 5 files changed, 120 insertions(+), 1 deletion(-) > create mode 100644 drivers/media/rc/ir-bpf-decoder.c > > diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig > index eb2c3b6eca7f..10ad6167d87c 100644 > --- a/drivers/media/rc/Kconfig > +++ b/drivers/media/rc/Kconfig > @@ -120,6 +120,14 @@ config IR_IMON_DECODER > remote control and you would like to use it with a raw IR > receiver, or if you wish to use an encoder to transmit this IR. > > +config IR_BPF_DECODER > + bool "Enable IR raw decoder using BPF" > + depends on BPF_SYSCALL > + depends on RC_CORE=y > + help > +Enable this option to make it possible to load custom IR > +decoders written in BPF. > + > endif #RC_DECODERS > > menuconfig RC_DEVICES > diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile > index 2e1c87066f6c..12e1118430d0 100644 > --- a/drivers/media/rc/Makefile > +++ b/drivers/media/rc/Makefile > @@ -5,6 +5,7 @@ obj-y += keymaps/ > obj-$(CONFIG_RC_CORE) += rc-core.o > rc-core-y := rc-main.o rc-ir-raw.o > rc-core-$(CONFIG_LIRC) += lirc_dev.o > +rc-core-$(CONFIG_IR_BPF_DECODER) += ir-bpf-decoder.o -- ~Randy
Re: [PATCH 1/7] i2c: i2c-gpio: move header to platform_data
Hi Wolfram, On 15/05/18 07:37, Wolfram Sang wrote: arch/arm/mach-ks8695/board-acs5k.c | 2 +- arch/arm/mach-sa1100/simpad.c| 2 +- arch/mips/alchemy/board-gpr.c| 2 +- Those still need acks... diff --git a/arch/arm/mach-ks8695/board-acs5k.c b/arch/arm/mach-ks8695/board-acs5k.c index 937eb1d47e7b..ef835d82cdb9 100644 --- a/arch/arm/mach-ks8695/board-acs5k.c +++ b/arch/arm/mach-ks8695/board-acs5k.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include ... diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c index ace010479eb6..49a61e6f3c5f 100644 --- a/arch/arm/mach-sa1100/simpad.c +++ b/arch/arm/mach-sa1100/simpad.c @@ -37,7 +37,7 @@ #include #include #include -#include +#include #include "generic.h" diff --git a/arch/mips/alchemy/board-gpr.c b/arch/mips/alchemy/board-gpr.c index 4e79dbd54a33..fa75d75b5ba9 100644 --- a/arch/mips/alchemy/board-gpr.c +++ b/arch/mips/alchemy/board-gpr.c @@ -29,7 +29,7 @@ #include #include #include -#include +#include #include #include #include ... and this was the shortened diff for those. Greg, Russell, Ralf, James? Is it okay if I take this via my tree? Yes, I have no problem with that for the ks8695 part. Acked-by: Greg Ungerer Thanks Greg Thanks, Wolfram
[RFC PATCH 0/5] Add ChromeOS EC CEC Support
Hi All, The new Google "Fizz" Intel-based ChromeOS device is gaining CEC support throught it's Embedded Controller, to enable the Linux CEC Core to communicate with it and get the CEC Physical Address from the correct HDMI Connector, the following must be added/changed: - Add the CEC sub-device registration in the ChromeOS EC MFD Driver - Add the CEC related commands and events definitions into the EC MFD driver - Add a way to get a CEC notifier with it's (optional) connector name - Add the CEC notifier to the i915 HDMI driver - Add the proper ChromeOS EC CEC Driver The CEC notifier with the connector name is the tricky point, since even on Device-Tree platforms, there is no way to distinguish between multiple HDMI connectors from the same DRM driver. The solution I implemented is pretty simple and only adds an optional connector name to eventually distinguish an HDMI connector notifier from another if they share the same device. Feel free to comment this patchset ! Neil Armstrong (5): mfd: cros_ec_dev: Add CEC sub-device registration media: cec-notifier: Get notifier by device and connector name drm/i915: hdmi: add CEC notifier to intel_hdmi mfd: cros-ec: Introduce CEC commands and events definitions. media: platform: Add Chrome OS EC CEC driver drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_hdmi.c| 10 + drivers/media/cec/cec-notifier.c | 30 ++- drivers/media/platform/Kconfig | 11 + drivers/media/platform/Makefile | 2 + drivers/media/platform/cros-ec/Makefile | 1 + drivers/media/platform/cros-ec/cros-ec-cec.c | 331 +++ drivers/mfd/cros_ec_dev.c| 16 ++ drivers/platform/chrome/cros_ec_proto.c | 42 +++- include/linux/mfd/cros_ec.h | 2 +- include/linux/mfd/cros_ec_commands.h | 80 +++ include/media/cec-notifier.h | 44 +++- 12 files changed, 556 insertions(+), 15 deletions(-) create mode 100644 drivers/media/platform/cros-ec/Makefile create mode 100644 drivers/media/platform/cros-ec/cros-ec-cec.c -- 2.7.4
[RFC PATCH 1/5] mfd: cros_ec_dev: Add CEC sub-device registration
The EC can expose a CEC bus, thus add the cros-ec-cec MFD sub-device when the CEC feature bit is present. Signed-off-by: Neil Armstrong --- drivers/mfd/cros_ec_dev.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c index eafd06f..57064ec 100644 --- a/drivers/mfd/cros_ec_dev.c +++ b/drivers/mfd/cros_ec_dev.c @@ -383,6 +383,18 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec) kfree(msg); } +static void cros_ec_cec_register(struct cros_ec_dev *ec) +{ + int ret; + struct mfd_cell cec_cell = { + .name = "cros-ec-cec", + }; + + ret = mfd_add_devices(ec->dev, 0, &cec_cell, 1, NULL, 0, NULL); + if (ret) + dev_err(ec->dev, "failed to add EC CEC\n"); +} + static int ec_device_probe(struct platform_device *pdev) { int retval = -ENOMEM; @@ -422,6 +434,10 @@ static int ec_device_probe(struct platform_device *pdev) if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE)) cros_ec_sensors_register(ec); + /* check whether this EC handles CEC. */ + if (cros_ec_check_features(ec, EC_FEATURE_CEC)) + cros_ec_cec_register(ec); + /* Take control of the lightbar from the EC. */ lb_manual_suspend_ctrl(ec, 1); -- 2.7.4
[RFC PATCH 4/5] mfd: cros-ec: Introduce CEC commands and events definitions.
From: Stefan Adolfsson The EC can expose a CEC bus, this patch adds the CEC related definitions needed by the cros-ec-cec driver. Having a 16 byte mkbp event size makes it possible to send CEC messages from the EC to the AP directly inside the mkbp event instead of first doing a notification and then a read. Signed-off-by: Stefan Adolfsson Signed-off-by: Neil Armstrong --- drivers/platform/chrome/cros_ec_proto.c | 42 + include/linux/mfd/cros_ec.h | 2 +- include/linux/mfd/cros_ec_commands.h| 80 + 3 files changed, 114 insertions(+), 10 deletions(-) diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index e7bbdf9..ba47f79 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -504,29 +504,53 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev, } EXPORT_SYMBOL(cros_ec_cmd_xfer_status); +static int get_next_event_xfer(struct cros_ec_device *ec_dev, + struct cros_ec_command *msg, + int version, uint32_t size) +{ + int ret; + + msg->version = version; + msg->command = EC_CMD_GET_NEXT_EVENT; + msg->insize = size; + msg->outsize = 0; + + ret = cros_ec_cmd_xfer(ec_dev, msg); + if (ret > 0) { + ec_dev->event_size = ret - 1; + memcpy(&ec_dev->event_data, msg->data, size); + } + + return ret; +} + static int get_next_event(struct cros_ec_device *ec_dev) { u8 buffer[sizeof(struct cros_ec_command) + sizeof(ec_dev->event_data)]; struct cros_ec_command *msg = (struct cros_ec_command *)&buffer; + static int cmd_version = 1; int ret; + BUILD_BUG_ON(sizeof(union ec_response_get_next_data_v1) != 16); + if (ec_dev->suspended) { dev_dbg(ec_dev->dev, "Device suspended.\n"); return -EHOSTDOWN; } - msg->version = 0; - msg->command = EC_CMD_GET_NEXT_EVENT; - msg->insize = sizeof(ec_dev->event_data); - msg->outsize = 0; + if (cmd_version == 1) { + ret = get_next_event_xfer(ec_dev, msg, cmd_version, + sizeof(ec_dev->event_data)); + if (ret != EC_RES_INVALID_VERSION) + return ret; - ret = cros_ec_cmd_xfer(ec_dev, msg); - if (ret > 0) { - ec_dev->event_size = ret - 1; - memcpy(&ec_dev->event_data, msg->data, - sizeof(ec_dev->event_data)); + /* Fallback to version 0 for future send attempts */ + cmd_version = 0; } + ret = get_next_event_xfer(ec_dev, msg, cmd_version, + sizeof(struct ec_response_get_next_event)); + return ret; } diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h index 2d4e23c..f3415eb 100644 --- a/include/linux/mfd/cros_ec.h +++ b/include/linux/mfd/cros_ec.h @@ -147,7 +147,7 @@ struct cros_ec_device { bool mkbp_event_supported; struct blocking_notifier_head event_notifier; - struct ec_response_get_next_event event_data; + struct ec_response_get_next_event_v1 event_data; int event_size; u32 host_event_wake_mask; }; diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h index f2edd99..18df466 100644 --- a/include/linux/mfd/cros_ec_commands.h +++ b/include/linux/mfd/cros_ec_commands.h @@ -804,6 +804,8 @@ enum ec_feature_code { EC_FEATURE_MOTION_SENSE_FIFO = 24, /* EC has RTC feature that can be controlled by host commands */ EC_FEATURE_RTC = 27, + /* EC supports CEC commands */ + EC_FEATURE_CEC = 35, }; #define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32)) @@ -2078,6 +2080,12 @@ enum ec_mkbp_event { /* EC sent a sysrq command */ EC_MKBP_EVENT_SYSRQ = 6, + /* Notify the AP that something happened on CEC */ + EC_MKBP_CEC_EVENT = 8, + + /* Send an incoming CEC message to the AP */ + EC_MKBP_EVENT_CEC_MESSAGE = 9, + /* Number of MKBP events */ EC_MKBP_EVENT_COUNT, }; @@ -2093,12 +2101,31 @@ union ec_response_get_next_data { uint32_t sysrq; } __packed; +union ec_response_get_next_data_v1 { + uint8_t key_matrix[16]; + + /* Unaligned */ + uint32_t host_event; + + uint32_t buttons; + uint32_t switches; + uint32_t sysrq; + uint32_t cec_events; + uint8_tcec_message[16]; +} __packed; + struct ec_response_get_next_event { uint8_t event_type; /* Followed by event data if any */ union ec_response_get_next_data data; } __packed; +struct ec_response_get_next_event_v1 { + uint8_t event_type; + /* Followed by event data if any */ + union ec_respons
[RFC PATCH 3/5] drm/i915: hdmi: add CEC notifier to intel_hdmi
This patchs adds the cec_notifier feature to the intel_hdmi part of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate between each HDMI ports. The changes will allow the i915 HDMI code to notify EDID and HPD changes to an eventual CEC adapter. Signed-off-by: Neil Armstrong --- drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_hdmi.c | 10 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d436858..b50e51b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -39,6 +39,7 @@ #include #include #include +#include /** * __wait_for - magic wait macro @@ -1001,6 +1002,7 @@ struct intel_hdmi { bool has_audio; bool rgb_quant_range_selectable; struct intel_connector *attached_connector; + struct cec_notifier *notifier; }; struct intel_dp_mst_encoder; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 1baef4a..9b94d72 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1868,6 +1868,8 @@ intel_hdmi_set_edid(struct drm_connector *connector) connected = true; } + cec_notifier_set_phys_addr_from_edid(intel_hdmi->notifier, edid); + return connected; } @@ -1876,6 +1878,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) { enum drm_connector_status status; struct drm_i915_private *dev_priv = to_i915(connector->dev); + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); @@ -1891,6 +1894,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); + if (status != connector_status_connected) + cec_notifier_phys_addr_invalidate(intel_hdmi->notifier); + return status; } @@ -2358,6 +2364,10 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, u32 temp = I915_READ(PEG_BAND_GAP_DATA); I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd); } + + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name); + if (!intel_hdmi->notifier) + DRM_DEBUG_KMS("CEC notifier get failed\n"); } void intel_hdmi_init(struct drm_i915_private *dev_priv, -- 2.7.4
[RFC PATCH 5/5] media: platform: Add Chrome OS EC CEC driver
The Chrome OS Embedded Controller can expose a CEC bus, this patch add the driver for such feature of the Embedded Controller. This driver is part of the cros-ec MFD and will be add as a sub-device when the feature bit is exposed by the EC. The controller will only handle a single logical address and handles all the messages retries and will only expose Success or Error. When the logical address is invalid, the controller will act as a CEC sniffer and transfer all messages on the bus. Signed-off-by: Neil Armstrong --- drivers/media/platform/Kconfig | 11 + drivers/media/platform/Makefile | 2 + drivers/media/platform/cros-ec/Makefile | 1 + drivers/media/platform/cros-ec/cros-ec-cec.c | 331 +++ 4 files changed, 345 insertions(+) create mode 100644 drivers/media/platform/cros-ec/Makefile create mode 100644 drivers/media/platform/cros-ec/cros-ec-cec.c diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index c7a1cf8..e55a8ed2 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -546,6 +546,17 @@ menuconfig CEC_PLATFORM_DRIVERS if CEC_PLATFORM_DRIVERS +config VIDEO_CROS_EC_CEC + tristate "Chrome OS EC CEC driver" + depends on MFD_CROS_EC || COMPILE_TEST + select CEC_CORE + select CEC_NOTIFIER + ---help--- + If you say yes here you will get support for the + Chrome OS Embedded Controller's CEC. + The CEC bus is present in the HDMI connector and enables communication + between compatible devices. + config VIDEO_MESON_AO_CEC tristate "Amlogic Meson AO CEC driver" depends on ARCH_MESON || COMPILE_TEST diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 932515d..0e0582e 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -92,3 +92,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= qcom/camss-8x16/ obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/ obj-y += meson/ + +obj-y += cros-ec/ diff --git a/drivers/media/platform/cros-ec/Makefile b/drivers/media/platform/cros-ec/Makefile new file mode 100644 index 000..9ce97f9 --- /dev/null +++ b/drivers/media/platform/cros-ec/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_VIDEO_CROS_EC_CEC) += cros-ec-cec.o diff --git a/drivers/media/platform/cros-ec/cros-ec-cec.c b/drivers/media/platform/cros-ec/cros-ec-cec.c new file mode 100644 index 000..fea90da --- /dev/null +++ b/drivers/media/platform/cros-ec/cros-ec-cec.c @@ -0,0 +1,331 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * CEC driver for Chrome OS Embedded Controller + * + * Copyright (c) 2018 BayLibre, SAS + * Author: Neil Armstrong + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRV_NAME "cros-ec-cec" + +/** + * struct cros_ec_cec - Driver data for EC CEC + * + * @cros_ec: Pointer to EC device + * @notifier: Notifier info for responding to EC events + * @adap: CEC adapter + * @notify: CEC notifier pointer + * @rc_msg: storage for a received message + */ +struct cros_ec_cec { + struct cros_ec_device *cros_ec; + struct notifier_block notifier; + struct cec_adapter *adap; + struct cec_notifier *notify; + struct cec_msg rx_msg; +}; + +static void handle_cec_message(struct cros_ec_cec *cros_ec_cec) +{ + struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec; + uint8_t *cec_message = cros_ec->event_data.data.cec_message; + unsigned int len = cros_ec->event_size; + + cros_ec_cec->rx_msg.len = len; + memcpy(cros_ec_cec->rx_msg.msg, cec_message, len); + + cec_received_msg(cros_ec_cec->adap, &cros_ec_cec->rx_msg); +} + +static void handle_cec_event(struct cros_ec_cec *cros_ec_cec) +{ + struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec; + uint32_t events = cros_ec->event_data.data.cec_events; + + if (events & EC_MKBP_CEC_SEND_OK) + cec_transmit_attempt_done(cros_ec_cec->adap, + CEC_TX_STATUS_OK); + + if (events & EC_MKBP_CEC_SEND_FAILED) + cec_transmit_attempt_done(cros_ec_cec->adap, + CEC_TX_STATUS_ERROR); +} + +static int cros_ec_cec_event(struct notifier_block *nb, + unsigned long queued_during_suspend, void *_notify) +{ + struct cros_ec_cec *cros_ec_cec; + struct cros_ec_device *cros_ec; + + cros_ec_cec = container_of(nb, struct cros_ec_cec, notifier); + cros_ec = cros_ec_cec->cros_ec; + + if (cros_ec->event_data.event_type == EC_MKBP_CEC_EVENT) { + handle_cec_event(cros_ec_cec); + return NOTIFY_OK; + } + + if (cros_ec->event_data.event_type == EC_MKBP_EVENT_CEC_MESSAGE) { + handle_cec_message(cr
[RFC PATCH 2/5] media: cec-notifier: Get notifier by device and connector name
In non device-tree world, we can need to get the notifier by the driver name directly and eventually defer probe if not yet created. This patch adds a variant of the get function by using the device name instead and will not create a notifier if not yet created. But the i915 driver exposes at least 2 HDMI connectors, this patch also adds the possibility to add a connector name tied to the notifier device to form a tuple and associate different CEC controllers for each HDMI connectors. Signed-off-by: Neil Armstrong --- drivers/media/cec/cec-notifier.c | 30 --- include/media/cec-notifier.h | 44 ++-- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c index 16dffa0..716070a 100644 --- a/drivers/media/cec/cec-notifier.c +++ b/drivers/media/cec/cec-notifier.c @@ -21,6 +21,7 @@ struct cec_notifier { struct list_head head; struct kref kref; struct device *dev; + const char *conn; struct cec_adapter *cec_adap; void (*callback)(struct cec_adapter *adap, u16 pa); @@ -30,13 +31,34 @@ struct cec_notifier { static LIST_HEAD(cec_notifiers); static DEFINE_MUTEX(cec_notifiers_lock); -struct cec_notifier *cec_notifier_get(struct device *dev) +struct cec_notifier *cec_notifier_get_byname(const char *name, +const char *conn) { struct cec_notifier *n; mutex_lock(&cec_notifiers_lock); list_for_each_entry(n, &cec_notifiers, head) { - if (n->dev == dev) { + if (!strcmp(dev_name(n->dev), name) && + (!conn || !strcmp(n->conn, conn))) { + kref_get(&n->kref); + mutex_unlock(&cec_notifiers_lock); + return n; + } + } + mutex_unlock(&cec_notifiers_lock); + + return NULL; +} +EXPORT_SYMBOL_GPL(cec_notifier_get_byname); + +struct cec_notifier *cec_notifier_get_conn(struct device *dev, const char *conn) +{ + struct cec_notifier *n; + + mutex_lock(&cec_notifiers_lock); + list_for_each_entry(n, &cec_notifiers, head) { + if (n->dev == dev && + (!conn || !strcmp(n->conn, conn))) { kref_get(&n->kref); mutex_unlock(&cec_notifiers_lock); return n; @@ -46,6 +68,8 @@ struct cec_notifier *cec_notifier_get(struct device *dev) if (!n) goto unlock; n->dev = dev; + if (conn) + n->conn = devm_kstrdup(dev, conn, GFP_KERNEL); n->phys_addr = CEC_PHYS_ADDR_INVALID; mutex_init(&n->lock); kref_init(&n->kref); @@ -54,7 +78,7 @@ struct cec_notifier *cec_notifier_get(struct device *dev) mutex_unlock(&cec_notifiers_lock); return n; } -EXPORT_SYMBOL_GPL(cec_notifier_get); +EXPORT_SYMBOL_GPL(cec_notifier_get_conn); static void cec_notifier_release(struct kref *kref) { diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h index cf0add7..70f2974 100644 --- a/include/media/cec-notifier.h +++ b/include/media/cec-notifier.h @@ -20,6 +20,37 @@ struct cec_notifier; #if IS_REACHABLE(CONFIG_CEC_CORE) && IS_ENABLED(CONFIG_CEC_NOTIFIER) /** + * cec_notifier_get_byname - find a cec_notifier for the given device name + * and connector tuple. + * @name: device name that sends the events. + * @conn: the connector name from which the event occurs + * + * If a notifier for device @name exists, then increase the refcount and + * return that notifier. + * + * If it doesn't exist, return NULL + */ +struct cec_notifier *cec_notifier_get_byname(const char *name, +const char *conn); + +/** + * cec_notifier_get_conn - find or create a new cec_notifier for the given + * device and connector tuple. + * @dev: device that sends the events. + * @conn: the connector name from which the event occurs + * + * If a notifier for device @dev already exists, then increase the refcount + * and return that notifier. + * + * If it doesn't exist, then allocate a new notifier struct and return a + * pointer to that new struct. + * + * Return NULL if the memory could not be allocated. + */ +struct cec_notifier *cec_notifier_get_conn(struct device *dev, + const char *conn); + +/** * cec_notifier_get - find or create a new cec_notifier for the given device. * @dev: device that sends the events. * @@ -31,7 +62,10 @@ struct cec_notifier; * * Return NULL if the memory could not be allocated. */ -struct cec_notifier *cec_notifier_get(struct device *dev); +static inline struct cec_notifier *cec_notifier_get(struct device *dev) +{ + return cec_notifier_get_conn(dev, NULL); +} /** * cec_notifier_put - decrease refcount and delete when the refcount reaches 0. @@
Re: [PATCH 2/2] media: i2c: mt9t112: Add device tree support
Hi Jacopo, On Mon, May 14, 2018 at 04:30:44PM +0200, jacopo mondi wrote: > Hi Sakari, > > On Mon, May 07, 2018 at 12:32:19PM +0300, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Wed, Apr 25, 2018 at 01:00:14PM +0200, Jacopo Mondi wrote: > > [snip] > > > > static int mt9t112_probe(struct i2c_client *client, > > >const struct i2c_device_id *did) > > > { > > > struct mt9t112_priv *priv; > > > int ret; > > > > > > - if (!client->dev.platform_data) { > > > + if (!client->dev.of_node && !client->dev.platform_data) { > > > dev_err(&client->dev, "mt9t112: missing platform data!\n"); > > > return -EINVAL; > > > } > > > @@ -1081,23 +1118,39 @@ static int mt9t112_probe(struct i2c_client > > > *client, > > > if (!priv) > > > return -ENOMEM; > > > > > > - priv->info = client->dev.platform_data; > > > priv->init_done = false; > > > - > > > - v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops); > > > - > > > - priv->clk = devm_clk_get(&client->dev, "extclk"); > > > - if (PTR_ERR(priv->clk) == -ENOENT) { > > > + priv->dev = &client->dev; > > > + > > > + if (client->dev.platform_data) { > > > + priv->info = client->dev.platform_data; > > > + > > > + priv->clk = devm_clk_get(&client->dev, "extclk"); > > > > extclk needs to be documented in DT binding documentation. > > > > > + if (PTR_ERR(priv->clk) == -ENOENT) { > > > + priv->clk = NULL; > > > + } else if (IS_ERR(priv->clk)) { > > > + dev_err(&client->dev, > > > + "Unable to get clock \"extclk\"\n"); > > > + return PTR_ERR(priv->clk); > > > + } > > > + } else { > > > + /* > > > + * External clock frequencies != 24MHz are only supported > > > + * for non-OF systems. > > > + */ > > > > Shouldn't you actually set the frequency? Or perhaps even better to check > > it, and use assigned-clocks and assigned-clock-rates properties? > > > > I might be confused, but my intention was to use an external clock > reference, with a configurable frequency only in the platform data use > case. As you can see in this 'else' branch, in OF case, the priv->clk > field is null, and all the PLL and clock computations are performed > assuming a 24MHz input clock. > > In my opinion, as the driver when running on OF systems does not > get any reference to 'extclk' clock, it should not be documented in > bindings. Do you agree? Uh, isn't the clock generally controlled by the driver on OF-based systems? You could assign the frequency in DT though, and not in the driver, but that should be documented in binding documentation. The register configuration the driver does not appear to be dependent on the clock frequency, which suggests that it is only applicable to a particular frequency --- 24 MHz? > > Thanks >j > > > > priv->clk = NULL; > > > - } else if (IS_ERR(priv->clk)) { > > > - dev_err(&client->dev, "Unable to get clock \"extclk\"\n"); > > > - return PTR_ERR(priv->clk); > > > + priv->info = &mt9t112_default_pdata_24MHz; > > > + > > > + ret = mt9t112_parse_dt(priv); > > > + if (ret) > > > + return ret; > > > } > > > > > > - priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby", > > > + v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops); > > > + > > > + priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "powerdown", > > >GPIOD_OUT_HIGH); > > > if (IS_ERR(priv->standby_gpio)) { > > > - dev_err(&client->dev, "Unable to get gpio \"standby\"\n"); > > > + dev_err(&client->dev, "Unable to get gpio \"powerdown\"\n"); > > > return PTR_ERR(priv->standby_gpio); > > > } > > > > > > @@ -1124,9 +1177,19 @@ static const struct i2c_device_id mt9t112_id[] = { > > > }; > > > MODULE_DEVICE_TABLE(i2c, mt9t112_id); > > > > > > +#if IS_ENABLED(CONFIG_OF) > > > +static const struct of_device_id mt9t112_of_match[] = { > > > + { .compatible = "micron,mt9t111", }, > > > + { .compatible = "micron,mt9t112", }, > > > + { /* sentinel */ }, > > > +}; > > > +MODULE_DEVICE_TABLE(of, mt9t112_of_match); > > > +#endif > > > + > > > static struct i2c_driver mt9t112_i2c_driver = { > > > .driver = { > > > .name = "mt9t112", > > > + .of_match_table = of_match_ptr(mt9t112_of_match), > > > > No need to use of_match_ptr(). > > > > > }, > > > .probe= mt9t112_probe, > > > .remove = mt9t112_remove, > > > > -- > > Sakari Ailus > > e-mail: sakari.ai...@iki.fi -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH 1/7] i2c: i2c-gpio: move header to platform_data
On Mon, May 14, 2018 at 11:37:20PM +0200, Wolfram Sang wrote: > > diff --git a/arch/mips/alchemy/board-gpr.c b/arch/mips/alchemy/board-gpr.c > > index 4e79dbd54a33..fa75d75b5ba9 100644 > > --- a/arch/mips/alchemy/board-gpr.c > > +++ b/arch/mips/alchemy/board-gpr.c > > @@ -29,7 +29,7 @@ > > #include > > #include > > #include > > -#include > > +#include > > #include > > #include > > #include Acked-by: James Hogan Cheers James signature.asc Description: PGP signature
Re: [PATCH 1/7] i2c: i2c-gpio: move header to platform_data
> arch/arm/mach-ks8695/board-acs5k.c | 2 +- > arch/arm/mach-sa1100/simpad.c| 2 +- > arch/mips/alchemy/board-gpr.c| 2 +- Those still need acks... > diff --git a/arch/arm/mach-ks8695/board-acs5k.c > b/arch/arm/mach-ks8695/board-acs5k.c > index 937eb1d47e7b..ef835d82cdb9 100644 > --- a/arch/arm/mach-ks8695/board-acs5k.c > +++ b/arch/arm/mach-ks8695/board-acs5k.c > @@ -19,7 +19,7 @@ > #include > #include > #include > -#include > +#include > #include > > #include ... > diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c > index ace010479eb6..49a61e6f3c5f 100644 > --- a/arch/arm/mach-sa1100/simpad.c > +++ b/arch/arm/mach-sa1100/simpad.c > @@ -37,7 +37,7 @@ > #include > #include > #include > -#include > +#include > > #include "generic.h" > > diff --git a/arch/mips/alchemy/board-gpr.c b/arch/mips/alchemy/board-gpr.c > index 4e79dbd54a33..fa75d75b5ba9 100644 > --- a/arch/mips/alchemy/board-gpr.c > +++ b/arch/mips/alchemy/board-gpr.c > @@ -29,7 +29,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include ... and this was the shortened diff for those. Greg, Russell, Ralf, James? Is it okay if I take this via my tree? Thanks, Wolfram signature.asc Description: PGP signature
Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error
On Mon, 2018-05-14 at 18:48 +0200, Daniel Vetter wrote: > On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote: > > Quoting Ezequiel Garcia (2018-05-09 21:14:49) > > > Change how dma_fence_add_callback() behaves, when the fence > > > has error-signaled by the time it is being add. After this commit, > > > dma_fence_add_callback() returns the fence error, if it > > > has error-signaled before dma_fence_add_callback() is called. > > > > Why? What problem are you trying to solve? fence->error does not imply > > that the fence has yet been signaled, and the caller wants a callback > > when it is signaled. > > On top this is incosistent, e.g. we don't do the same for any of the other > dma_fence interfaces. Plus there's the issue that you might alias errno > values with fence errno values. > Right. > I think keeping the error codes from the functions you're calling distinct > from the error code of the fence itself makes a lot of sense. The first > tells you whether your request worked out (or why not), the second tells > you whether the asynchronous dma operation (gpu rendering, page flip, > whatever) that the dma_fence represents worked out (or why not). That's 2 > distinct things imo. > > Might be good to show us the driver code that needs this behaviour so we > can discuss how to best handle your use-case. > This change arose while discussing the in-fences support for video4linux. Here's the patch that calls dma_fence_add_callback https://lkml.org/lkml/2018/5/4/766. The code snippet currently looks something like this: if (vb->in_fence) { ret = dma_fence_add_callback(vb->in_fence, &vb->fence_cb, vb2_qbuf_fence_cb); /* is the fence signaled? */ if (ret == -ENOENT) { dma_fence_put(vb->in_fence); vb->in_fence = NULL; } else if (ret) { goto unlock; } } In this use case, if the callback is added successfully, the video4linux core defers the activation of the buffer until the fence signals. If the fence is signaled (currently disregarding of errors) then the buffer is assumed to be ready to be activated, and so it gets queued for hardware usage. Giving some more thought to this, I'm not so sure what is the right action if a fence signaled with error. In this case, it appears to me that we shouldn't be using this buffer if its in-fence is in error, but perhaps I'm missing something. Thanks! Eze
[PATCH v1 0/4] IR decoding using BPF
The kernel IR decoders support the most widely used IR protocols, but there are many protocols which are not supported[1]. For example, the lirc-remotes[2] repo has over 2700 remotes, many of which are not supported by rc-core. There is a "long tail" of unsupported IR protocols. IR encoding is done in such a way that some simple circuit can decode it; therefore, bpf is ideal. In order to support all these protocols, here we have bpf based IR decoding. The idea is that user-space can define a decoder in bpf, attach it to the rc device through the lirc chardev. Separate work is underway to extend ir-keytable to have an extensive library of bpf-based decoders, and a much expanded library of rc keymaps. Another future application would be to compile IRP[3] to a IR BPF program, and so support virtually every remote without having to write a decoder for each. Thanks, Sean Young [1] http://www.hifi-remote.com/wiki/index.php?title=DecodeIR [2] https://sourceforge.net/p/lirc-remotes/code/ci/master/tree/remotes/ [3] http://www.hifi-remote.com/wiki/index.php?title=IRP_Notation Sean Young (4): media: rc: introduce BPF_PROG_IR_DECODER media: bpf: allow raw IR decoder bpf programs to be used media: rc bpf: move ir_raw_event to uapi samples/bpf: an example of a raw IR decoder drivers/media/rc/Kconfig | 8 + drivers/media/rc/Makefile | 1 + drivers/media/rc/ir-bpf-decoder.c | 284 ++ drivers/media/rc/lirc_dev.c | 30 +++ drivers/media/rc/rc-core-priv.h | 15 ++ drivers/media/rc/rc-ir-raw.c | 5 + include/linux/bpf_types.h | 3 + include/media/rc-core.h | 19 +- include/uapi/linux/bpf.h | 17 +- include/uapi/linux/bpf_rcdev.h| 24 ++ kernel/bpf/syscall.c | 7 + samples/bpf/Makefile | 4 + samples/bpf/bpf_load.c| 9 +- samples/bpf/grundig_decoder_kern.c| 112 + samples/bpf/grundig_decoder_user.c| 54 tools/bpf/bpftool/prog.c | 1 + tools/include/uapi/linux/bpf.h| 17 +- tools/testing/selftests/bpf/bpf_helpers.h | 6 + 18 files changed, 594 insertions(+), 22 deletions(-) create mode 100644 drivers/media/rc/ir-bpf-decoder.c create mode 100644 include/uapi/linux/bpf_rcdev.h create mode 100644 samples/bpf/grundig_decoder_kern.c create mode 100644 samples/bpf/grundig_decoder_user.c -- 2.17.0
[PATCH v1 1/4] media: rc: introduce BPF_PROG_IR_DECODER
Add support for BPF_PROG_IR_DECODER. This type of BPF program can call rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report that the last key should be repeated. Signed-off-by: Sean Young --- drivers/media/rc/Kconfig | 8 +++ drivers/media/rc/Makefile | 1 + drivers/media/rc/ir-bpf-decoder.c | 93 +++ include/linux/bpf_types.h | 3 + include/uapi/linux/bpf.h | 16 +- 5 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 drivers/media/rc/ir-bpf-decoder.c diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig index eb2c3b6eca7f..10ad6167d87c 100644 --- a/drivers/media/rc/Kconfig +++ b/drivers/media/rc/Kconfig @@ -120,6 +120,14 @@ config IR_IMON_DECODER remote control and you would like to use it with a raw IR receiver, or if you wish to use an encoder to transmit this IR. +config IR_BPF_DECODER + bool "Enable IR raw decoder using BPF" + depends on BPF_SYSCALL + depends on RC_CORE=y + help + Enable this option to make it possible to load custom IR + decoders written in BPF. + endif #RC_DECODERS menuconfig RC_DEVICES diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile index 2e1c87066f6c..12e1118430d0 100644 --- a/drivers/media/rc/Makefile +++ b/drivers/media/rc/Makefile @@ -5,6 +5,7 @@ obj-y += keymaps/ obj-$(CONFIG_RC_CORE) += rc-core.o rc-core-y := rc-main.o rc-ir-raw.o rc-core-$(CONFIG_LIRC) += lirc_dev.o +rc-core-$(CONFIG_IR_BPF_DECODER) += ir-bpf-decoder.o obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o obj-$(CONFIG_IR_RC6_DECODER) += ir-rc6-decoder.o diff --git a/drivers/media/rc/ir-bpf-decoder.c b/drivers/media/rc/ir-bpf-decoder.c new file mode 100644 index ..aaa5e208b1a5 --- /dev/null +++ b/drivers/media/rc/ir-bpf-decoder.c @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: GPL-2.0 +// ir-bpf-decoder.c - handles bpf decoders +// +// Copyright (C) 2018 Sean Young + +#include +#include +#include "rc-core-priv.h" + +/* + * BPF interface for raw IR decoder + */ +const struct bpf_prog_ops ir_decoder_prog_ops = { +}; + +BPF_CALL_1(bpf_rc_repeat, struct ir_raw_event*, event) +{ + struct ir_raw_event_ctrl *ctrl; + + ctrl = container_of(event, struct ir_raw_event_ctrl, prev_ev); + + rc_repeat(ctrl->dev); + return 0; +} + +static const struct bpf_func_proto rc_repeat_proto = { + .func = bpf_rc_repeat, + .gpl_only = true, // rc_repeat is EXPORT_SYMBOL_GPL + .ret_type = RET_VOID, + .arg1_type = ARG_PTR_TO_CTX, +}; + +BPF_CALL_4(bpf_rc_keydown, struct ir_raw_event*, event, u32, protocol, + u32, scancode, u32, toggle) +{ + struct ir_raw_event_ctrl *ctrl; + + ctrl = container_of(event, struct ir_raw_event_ctrl, prev_ev); + rc_keydown(ctrl->dev, protocol, scancode, toggle != 0); + return 0; +} + +static const struct bpf_func_proto rc_keydown_proto = { + .func = bpf_rc_keydown, + .gpl_only = true, // rc_keydown is EXPORT_SYMBOL_GPL + .ret_type = RET_VOID, + .arg1_type = ARG_PTR_TO_CTX, + .arg2_type = ARG_ANYTHING, + .arg3_type = ARG_ANYTHING, + .arg4_type = ARG_ANYTHING, +}; + +static const struct bpf_func_proto *ir_decoder_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) +{ + switch (func_id) { + case BPF_FUNC_rc_repeat: + return &rc_repeat_proto; + case BPF_FUNC_rc_keydown: + return &rc_keydown_proto; + case BPF_FUNC_map_lookup_elem: + return &bpf_map_lookup_elem_proto; + case BPF_FUNC_map_update_elem: + return &bpf_map_update_elem_proto; + case BPF_FUNC_map_delete_elem: + return &bpf_map_delete_elem_proto; + case BPF_FUNC_ktime_get_ns: + return &bpf_ktime_get_ns_proto; + case BPF_FUNC_tail_call: + return &bpf_tail_call_proto; + case BPF_FUNC_get_prandom_u32: + return &bpf_get_prandom_u32_proto; + default: + return NULL; + } +} + +static bool ir_decoder_is_valid_access(int off, int size, + enum bpf_access_type type, + const struct bpf_prog *prog, + struct bpf_insn_access_aux *info) +{ + if (type == BPF_WRITE) + return false; + if (off < 0 || off + size > sizeof(struct ir_raw_event)) + return false; + + return true; +} + +const struct bpf_verifier_ops ir_decoder_verifier_ops = { + .get_func_proto = ir_decoder_func_proto, + .is_valid_access = ir_decoder_is_valid_access +}; diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h index 2b28fcf6f6ae..ee5355715ee0 100644 --- a/include/linux/bpf_types.h +++ b/include/linux/bpf_types.h @@ -25
[PATCH v1 3/4] media: rc bpf: move ir_raw_event to uapi
The context provided to a BPF_PROG_RAWIR_DECODER is a struct ir_raw_event; ensure user space has a a definition. Signed-off-by: Sean Young --- include/media/rc-core.h| 19 +-- include/uapi/linux/bpf_rcdev.h | 24 2 files changed, 25 insertions(+), 18 deletions(-) create mode 100644 include/uapi/linux/bpf_rcdev.h diff --git a/include/media/rc-core.h b/include/media/rc-core.h index 6742fd86ff65..5d31e31d8ade 100644 --- a/include/media/rc-core.h +++ b/include/media/rc-core.h @@ -21,6 +21,7 @@ #include #include #include +#include #include /** @@ -299,24 +300,6 @@ void rc_keydown_notimeout(struct rc_dev *dev, enum rc_proto protocol, void rc_keyup(struct rc_dev *dev); u32 rc_g_keycode_from_table(struct rc_dev *dev, u32 scancode); -/* - * From rc-raw.c - * The Raw interface is specific to InfraRed. It may be a good idea to - * split it later into a separate header. - */ -struct ir_raw_event { - union { - u32 duration; - u32 carrier; - }; - u8 duty_cycle; - - unsignedpulse:1; - unsignedreset:1; - unsignedtimeout:1; - unsignedcarrier_report:1; -}; - #define DEFINE_IR_RAW_EVENT(event) struct ir_raw_event event = {} static inline void init_ir_raw_event(struct ir_raw_event *ev) diff --git a/include/uapi/linux/bpf_rcdev.h b/include/uapi/linux/bpf_rcdev.h new file mode 100644 index ..d8629ff2b960 --- /dev/null +++ b/include/uapi/linux/bpf_rcdev.h @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* Copyright (c) 2018 Sean Young + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of version 2 of the GNU General Public + * License as published by the Free Software Foundation. + */ +#ifndef _UAPI__LINUX_BPF_RCDEV_H__ +#define _UAPI__LINUX_BPF_RCDEV_H__ + +struct ir_raw_event { + union { + __u32 duration; + __u32 carrier; + }; + __u8duty_cycle; + + unsignedpulse:1; + unsignedreset:1; + unsignedtimeout:1; + unsignedcarrier_report:1; +}; + +#endif /* _UAPI__LINUX_BPF_RCDEV_H__ */ -- 2.17.0
[PATCH v1 2/4] media: bpf: allow raw IR decoder bpf programs to be used
This implements attaching, detaching, querying and execution. The target fd has to be the /dev/lircN device. Signed-off-by: Sean Young --- drivers/media/rc/ir-bpf-decoder.c | 191 ++ drivers/media/rc/lirc_dev.c | 30 + drivers/media/rc/rc-core-priv.h | 15 +++ drivers/media/rc/rc-ir-raw.c | 5 + include/uapi/linux/bpf.h | 1 + kernel/bpf/syscall.c | 7 ++ 6 files changed, 249 insertions(+) diff --git a/drivers/media/rc/ir-bpf-decoder.c b/drivers/media/rc/ir-bpf-decoder.c index aaa5e208b1a5..651590a14772 100644 --- a/drivers/media/rc/ir-bpf-decoder.c +++ b/drivers/media/rc/ir-bpf-decoder.c @@ -91,3 +91,194 @@ const struct bpf_verifier_ops ir_decoder_verifier_ops = { .get_func_proto = ir_decoder_func_proto, .is_valid_access = ir_decoder_is_valid_access }; + +#define BPF_MAX_PROGS 64 + +int rc_dev_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog, u32 flags) +{ + struct ir_raw_event_ctrl *raw; + struct bpf_prog_array __rcu *old_array; + struct bpf_prog_array *new_array; + int ret; + + if (rcdev->driver_type != RC_DRIVER_IR_RAW) + return -EINVAL; + + ret = mutex_lock_interruptible(&rcdev->lock); + if (ret) + return ret; + + raw = rcdev->raw; + + if (raw->progs && bpf_prog_array_length(raw->progs) >= BPF_MAX_PROGS) { + ret = -E2BIG; + goto out; + } + + old_array = raw->progs; + ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array); + if (ret < 0) + goto out; + + rcu_assign_pointer(raw->progs, new_array); + bpf_prog_array_free(old_array); +out: + mutex_unlock(&rcdev->lock); + return ret; +} + +int rc_dev_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog, u32 flags) +{ + struct ir_raw_event_ctrl *raw; + struct bpf_prog_array __rcu *old_array; + struct bpf_prog_array *new_array; + int ret; + + if (rcdev->driver_type != RC_DRIVER_IR_RAW) + return -EINVAL; + + ret = mutex_lock_interruptible(&rcdev->lock); + if (ret) + return ret; + + raw = rcdev->raw; + + old_array = raw->progs; + ret = bpf_prog_array_copy(old_array, prog, NULL, &new_array); + if (ret < 0) { + bpf_prog_array_delete_safe(old_array, prog); + } else { + rcu_assign_pointer(raw->progs, new_array); + bpf_prog_array_free(old_array); + } + + bpf_prog_put(prog); + mutex_unlock(&rcdev->lock); + return 0; +} + +void rc_dev_bpf_run(struct rc_dev *rcdev) +{ + struct ir_raw_event_ctrl *raw = rcdev->raw; + + if (raw->progs) + BPF_PROG_RUN_ARRAY(raw->progs, &raw->prev_ev, BPF_PROG_RUN); +} + +void rc_dev_bpf_put(struct rc_dev *rcdev) +{ + struct bpf_prog_array *progs = rcdev->raw->progs; + int i, size; + + if (!progs) + return; + + size = bpf_prog_array_length(progs); + for (i = 0; i < size; i++) + bpf_prog_put(progs->progs[i]); + + bpf_prog_array_free(rcdev->raw->progs); +} + +int rc_dev_prog_attach(const union bpf_attr *attr) +{ + struct bpf_prog *prog; + struct rc_dev *rcdev; + int ret; + + if (attr->attach_flags & BPF_F_ALLOW_OVERRIDE) + return -EINVAL; + + prog = bpf_prog_get_type(attr->attach_bpf_fd, +BPF_PROG_TYPE_RAWIR_DECODER); + if (IS_ERR(prog)) + return PTR_ERR(prog); + + rcdev = rc_dev_get_from_fd(attr->target_fd); + if (IS_ERR(rcdev)) { + bpf_prog_put(prog); + return PTR_ERR(rcdev); + } + + ret = rc_dev_bpf_attach(rcdev, prog, attr->attach_flags); + if (ret) + bpf_prog_put(prog); + + put_device(&rcdev->dev); + + return ret; +} + +int rc_dev_prog_detach(const union bpf_attr *attr) +{ + struct bpf_prog *prog; + struct rc_dev *rcdev; + int ret; + + if (attr->attach_flags & BPF_F_ALLOW_OVERRIDE) + return -EINVAL; + + prog = bpf_prog_get_type(attr->attach_bpf_fd, +BPF_PROG_TYPE_RAWIR_DECODER); + if (IS_ERR(prog)) + return PTR_ERR(prog); + + rcdev = rc_dev_get_from_fd(attr->target_fd); + if (IS_ERR(rcdev)) { + bpf_prog_put(prog); + return PTR_ERR(rcdev); + } + + ret = rc_dev_bpf_detach(rcdev, prog, attr->attach_flags); + + bpf_prog_put(prog); + put_device(&rcdev->dev); + + return ret; +} + +int rc_dev_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr) +{ + __u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids); + struct bpf_prog_array *progs; + struct rc_dev *rcdev; + u32 cnt, flags = 0; + int ret; + + if (attr->query.query_
[PATCH v1 4/4] samples/bpf: an example of a raw IR decoder
This implements the grundig-16 IR protocol. Signed-off-by: Sean Young --- samples/bpf/Makefile | 4 + samples/bpf/bpf_load.c| 9 +- samples/bpf/grundig_decoder_kern.c| 112 ++ samples/bpf/grundig_decoder_user.c| 54 +++ tools/bpf/bpftool/prog.c | 1 + tools/include/uapi/linux/bpf.h| 17 +++- tools/testing/selftests/bpf/bpf_helpers.h | 6 ++ 7 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 samples/bpf/grundig_decoder_kern.c create mode 100644 samples/bpf/grundig_decoder_user.c diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 4d6a6edd4bf6..c6fa111f103a 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -44,6 +44,7 @@ hostprogs-y += xdp_monitor hostprogs-y += xdp_rxq_info hostprogs-y += syscall_tp hostprogs-y += cpustat +hostprogs-y += grundig_decoder # Libbpf dependencies LIBBPF := ../../tools/lib/bpf/bpf.o ../../tools/lib/bpf/nlattr.o @@ -95,6 +96,7 @@ xdp_monitor-objs := bpf_load.o $(LIBBPF) xdp_monitor_user.o xdp_rxq_info-objs := bpf_load.o $(LIBBPF) xdp_rxq_info_user.o syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o cpustat-objs := bpf_load.o $(LIBBPF) cpustat_user.o +grundig_decoder-objs := bpf_load.o $(LIBBPF) grundig_decoder_user.o # Tell kbuild to always build the programs always := $(hostprogs-y) @@ -148,6 +150,7 @@ always += xdp_rxq_info_kern.o always += xdp2skb_meta_kern.o always += syscall_tp_kern.o always += cpustat_kern.o +always += grundig_decoder_kern.o HOSTCFLAGS += -I$(objtree)/usr/include HOSTCFLAGS += -I$(srctree)/tools/lib/ @@ -193,6 +196,7 @@ HOSTLOADLIBES_xdp_monitor += -lelf HOSTLOADLIBES_xdp_rxq_info += -lelf HOSTLOADLIBES_syscall_tp += -lelf HOSTLOADLIBES_cpustat += -lelf +HOSTLOADLIBES_grundig_decoder += -lelf # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline: # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c index bebe4188b4b3..0fd389e95bb9 100644 --- a/samples/bpf/bpf_load.c +++ b/samples/bpf/bpf_load.c @@ -69,6 +69,7 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size) bool is_sockops = strncmp(event, "sockops", 7) == 0; bool is_sk_skb = strncmp(event, "sk_skb", 6) == 0; bool is_sk_msg = strncmp(event, "sk_msg", 6) == 0; + bool is_ir_decoder = strncmp(event, "ir_decoder", 10) == 0; size_t insns_cnt = size / sizeof(struct bpf_insn); enum bpf_prog_type prog_type; char buf[256]; @@ -102,6 +103,8 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size) prog_type = BPF_PROG_TYPE_SK_SKB; } else if (is_sk_msg) { prog_type = BPF_PROG_TYPE_SK_MSG; + } else if (is_ir_decoder) { + prog_type = BPF_PROG_TYPE_RAWIR_DECODER; } else { printf("Unknown event '%s'\n", event); return -1; @@ -116,7 +119,8 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size) prog_fd[prog_cnt++] = fd; - if (is_xdp || is_perf_event || is_cgroup_skb || is_cgroup_sk) + if (is_xdp || is_perf_event || is_cgroup_skb || is_cgroup_sk || + is_ir_decoder) return 0; if (is_socket || is_sockops || is_sk_skb || is_sk_msg) { @@ -607,7 +611,8 @@ static int do_load_bpf_file(const char *path, fixup_map_cb fixup_map) memcmp(shname, "cgroup/", 7) == 0 || memcmp(shname, "sockops", 7) == 0 || memcmp(shname, "sk_skb", 6) == 0 || - memcmp(shname, "sk_msg", 6) == 0) { + memcmp(shname, "sk_msg", 6) == 0 || + memcmp(shname, "ir_decoder", 10) == 0) { ret = load_and_attach(shname, data->d_buf, data->d_size); if (ret != 0) diff --git a/samples/bpf/grundig_decoder_kern.c b/samples/bpf/grundig_decoder_kern.c new file mode 100644 index ..c80f2c9cc69a --- /dev/null +++ b/samples/bpf/grundig_decoder_kern.c @@ -0,0 +1,112 @@ + +#include +#include +#include "bpf_helpers.h" +#include + +enum grundig_state { + STATE_INACTIVE, + STATE_HEADER_SPACE, + STATE_LEADING_PULSE, + STATE_BITS_SPACE, + STATE_BITS_PULSE, +}; + +struct decoder_state { + u32 bits; + enum grundig_state state; + u32 count; + u32 last_space; +}; + +struct bpf_map_def SEC("maps") decoder_state_map = { + .type = BPF_MAP_TYPE_ARRAY, + .key_size = sizeof(u32), + .value_size = sizeof(struct decoder_state), + .max_entries = 1, +}; + +#define US_TO_NS(t) 1000*(t) +static inline bool eq_margin(unsigned d1, unsigned d2, unsigned margin) +{ + return ((d1 > (d2 -
Re: [RFC PATCH 2/6] v4l2-core: push taking ioctl mutex down to ioctl handler.
On Mon, 2018-05-14 at 13:55 +0200, Hans Verkuil wrote: > From: Hans Verkuil > > The ioctl serialization mutex (vdev->lock or q->lock for vb2 queues) > was taken at the highest level in v4l2-dev.c. This prevents more > fine-grained locking since at that level we cannot examine the ioctl > arguments, we can only do that after video_usercopy is called. > > So push the locking down to __video_do_ioctl() and subdev_do_ioctl_lock(). > > This also allows us to make a few functions in v4l2-ioctl.c static and > video_usercopy() is no longer exported. > > The locking scheme is not changed by this patch, just pushed down. > > Signed-off-by: Hans Verkuil > --- > drivers/media/v4l2-core/v4l2-dev.c| 6 -- > drivers/media/v4l2-core/v4l2-ioctl.c | 17 ++--- > drivers/media/v4l2-core/v4l2-subdev.c | 17 - > include/media/v4l2-dev.h | 9 - > include/media/v4l2-ioctl.h| 12 > 5 files changed, 30 insertions(+), 31 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c > b/drivers/media/v4l2-core/v4l2-dev.c > index c4f4357e9ca4..4ffd7d60a901 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -360,14 +360,8 @@ static long v4l2_ioctl(struct file *filp, unsigned int > cmd, unsigned long arg) > int ret = -ENODEV; > > if (vdev->fops->unlocked_ioctl) { > - struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd); > - > - if (lock && mutex_lock_interruptible(lock)) > - return -ERESTARTSYS; > if (video_is_registered(vdev)) This is_registered check looks spurious. Other than that, it looks good.
Re: [PATCH v15 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
Hi Jacopo, Thanks for your feedback. On 2018-05-14 15:14:48 +0200, Jacopo Mondi wrote: > Hi Niklas, >thanks for the patch > > On Sun, May 13, 2018 at 09:19:17PM +0200, Niklas Söderlund wrote: > > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver > > supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are > > connected between the video sources and the video grabbers (VIN). > > > > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP. > > > > Signed-off-by: Niklas Söderlund > > > > --- > > > > * Changes since v14 > > - Data sheet update changed init sequence for PHY forcing a restructure > > of the driver. The restructure was so big I felt compel to drop all > > review tags :-( > > - The change was that the Renesas H3 procedure was aligned with other > > SoC in the Gen3 family procedure. I had kept the rework as separate > > patches and was planing to post once original driver with H3 and M3-W > > support where merged. As review tags are dropped I chosen to squash > > those patches into 2/2. > > - Add support for Gen3 V3M. > > - Add support for Gen3 M3-N. > > - Set PHTC_TESTCLR when stopping the PHY. > > - Revert back to the v12 and earlier phypll calculation as it turns out > > it was correct after all. > > > > * Changes since v13 > > - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i]. > > - Add define for PHCLM_STOPSTATECKL. > > - Update spelling in comments. > > - Update calculation in rcar_csi2_calc_phypll() according to > > https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one > > before v14 did not take into account that 2 bits per sample is > > transmitted. > > - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch > > statement to set correct number of lanes to enable. > > - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match > > style of rest of file. > > - Switch to %u instead of 0x%x when printing bus type. > > - Switch to %u instead of %d for priv->lanes which is unsigned. > > - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in > > rcar_csi2_formats[]. > > - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16. > > - Set INTSTATE after PL-11 is confirmed to match flow chart in > > datasheet. > > - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs. > > - Add Maxime's and laurent's tags. > > --- > > drivers/media/platform/rcar-vin/Kconfig | 12 + > > drivers/media/platform/rcar-vin/Makefile|1 + > > drivers/media/platform/rcar-vin/rcar-csi2.c | 1101 +++ > > 3 files changed, 1114 insertions(+) > > create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c > > > > diff --git a/drivers/media/platform/rcar-vin/Kconfig > > b/drivers/media/platform/rcar-vin/Kconfig > > index 8fa7ee468c63afb9..d5835da6d4100d87 100644 > > --- a/drivers/media/platform/rcar-vin/Kconfig > > +++ b/drivers/media/platform/rcar-vin/Kconfig > > @@ -1,3 +1,15 @@ > > +config VIDEO_RCAR_CSI2 > > + tristate "R-Car MIPI CSI-2 Receiver" > > + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF > > + depends on ARCH_RENESAS || COMPILE_TEST > > + select V4L2_FWNODE > > + help > > + Support for Renesas R-Car MIPI CSI-2 receiver. > > + Supports R-Car Gen3 SoCs. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called rcar-csi2. > > + > > config VIDEO_RCAR_VIN > > tristate "R-Car Video Input (VIN) Driver" > > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && > > MEDIA_CONTROLLER > > diff --git a/drivers/media/platform/rcar-vin/Makefile > > b/drivers/media/platform/rcar-vin/Makefile > > index 48c5632c21dc060b..5ab803d3e7c1aa57 100644 > > --- a/drivers/media/platform/rcar-vin/Makefile > > +++ b/drivers/media/platform/rcar-vin/Makefile > > @@ -1,3 +1,4 @@ > > rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o > > > > +obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o > > obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > > b/drivers/media/platform/rcar-vin/rcar-csi2.c > > new file mode 100644 > > index ..b19374f1516464dc > > --- /dev/null > > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > > @@ -0,0 +1,1101 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Renesas R-Car MIPI CSI-2 Receiver > > + * > > + * Copyright (C) 2018 Renesas Electronics Corp. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +struct rcar_csi2; > > + > > +/* Register offsets and bits */ > > + > > +/* Control Timing Select */ > > +#define TREF_REG 0x00 > > +#define TREF_TREF BIT(0) > > + > > +/* Software Reset */ > > +#define SRST_REG
Wine Enthusiasts List
Hi, Would you be interested in reaching out to "Wine Enthusiasts list " from USA? Our Databases:-1.Beer Drinkers List 2.Alcohol Drinkers List 3.Beverage Consumers 4.Liquor Drinkers List 5.Food Enthusiasts List 6.Gift Buyers List 7.Luxury Brand Buyers List 8.Home Owners List 9.Spa and Resort Visitors and many more. All the contacts are opt-in verified, 100 percent permission based and can be used for unlimited multi-channel marketing. Please let me know your thoughts towards procuring the Wine Enthusiasts list. Best Regards, Lori Grimes We respect your privacy, if you do not wish to receive any further emails from our end, please reply with a subject ³Leave Out².
RE: media: uvcvideo: Support realtek's UVC 1.5 device
> The length of UVC 1.5 video control is 48, and it id 34 for UVC 1.1. > Change it to 48 for UVC 1.5 device, > and the UVC 1.5 device can be recognized. > > More changes to the driver are needed for full UVC 1.5 compatibility. > However, at least the UVC 1.5 Realtek RTS5847/RTS5852 cameras have > been reported to work well. > > Signed-off-by: ming_qian > Tested-by: Kai-Heng Feng Hello! I have sucessfully tested this patch on Kernel 4.16.1 (Fedora 28) with Dell XPS 9370 using following device (output from lsusb): Bus 001 Device 002: ID 0bda:58f4 Realtek Semiconductor Corp. You can also find related dmesg output at https://bugs.launchpad.net/dell-sputnik/+bug/1763748/comments/35 Tested-by: Josef Šimánek > Reviewed-by: Hans de Goede > --- > drivers/media/usb/uvc/uvc_video.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_video.c > b/drivers/media/usb/uvc/uvc_video.c > index aa0082f..32dfb32 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -171,6 +171,8 @@ static int uvc_get_video_ctrl(struct uvc_streaming > *stream, > int ret; > > size = stream->dev->uvc_version >= 0x0110 ? 34 : 26; > + if (stream->dev->uvc_version >= 0x0150) > + size = 48; > if ((stream->dev->quirks & UVC_QUIRK_PROBE_DEF) && > query == UVC_GET_DEF) > return -EIO; > @@ -259,6 +261,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming > *stream, > int ret; > > size = stream->dev->uvc_version >= 0x0110 ? 34 : 26; > + if (stream->dev->uvc_version >= 0x0150) > + size = 48; > data = kzalloc(size, GFP_KERNEL); > if (data == NULL) > return -ENOMEM;
Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error
On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote: > Quoting Ezequiel Garcia (2018-05-09 21:14:49) > > Change how dma_fence_add_callback() behaves, when the fence > > has error-signaled by the time it is being add. After this commit, > > dma_fence_add_callback() returns the fence error, if it > > has error-signaled before dma_fence_add_callback() is called. > > Why? What problem are you trying to solve? fence->error does not imply > that the fence has yet been signaled, and the caller wants a callback > when it is signaled. On top this is incosistent, e.g. we don't do the same for any of the other dma_fence interfaces. Plus there's the issue that you might alias errno values with fence errno values. I think keeping the error codes from the functions you're calling distinct from the error code of the fence itself makes a lot of sense. The first tells you whether your request worked out (or why not), the second tells you whether the asynchronous dma operation (gpu rendering, page flip, whatever) that the dma_fence represents worked out (or why not). That's 2 distinct things imo. Might be good to show us the driver code that needs this behaviour so we can discuss how to best handle your use-case. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 5/7] imx: fix compiler warning
Acked-by: Steve Longerbeam On 05/14/2018 06:13 AM, Hans Verkuil wrote: From: Hans Verkuil drivers/staging/media/imx/imx-media-capture.c: In function 'vidioc_querycap': drivers/staging/media/imx/imx-media-capture.c:76:2: warning: 'strncpy' output truncated copying 15 bytes from a string of length 17 [-Wstringop-truncation] strncpy(cap->driver, "imx-media-capture", sizeof(cap->driver) - 1); ^~ Signed-off-by: Hans Verkuil Cc: Steve Longerbeam Cc: Philipp Zabel --- drivers/staging/media/imx/imx-media-capture.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c index 0ccabe04b0e1..4e3fdf8aeef5 100644 --- a/drivers/staging/media/imx/imx-media-capture.c +++ b/drivers/staging/media/imx/imx-media-capture.c @@ -73,8 +73,8 @@ static int vidioc_querycap(struct file *file, void *fh, { struct capture_priv *priv = video_drvdata(file); - strncpy(cap->driver, "imx-media-capture", sizeof(cap->driver) - 1); - strncpy(cap->card, "imx-media-capture", sizeof(cap->card) - 1); + strlcpy(cap->driver, "imx-media-capture", sizeof(cap->driver)); + strlcpy(cap->card, "imx-media-capture", sizeof(cap->card)); snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", priv->src_sd->name);
Re: Compressed formats - framed or unframed?
On 26 April 2018 at 17:25, Dave Stevenson wrote: > Hi All. > > I'm trying to get a V4L2 M2M driver sorted for the Raspberry Pi to > allow access to the video codecs. Much of it is working fine. > > One thing that isn't clear relates to video decode. Do the compressed > formats (eg V4L2_PIX_FMT_H264) have to be framed into one frame per > V4L2 buffer, or is providing unframed chunks of an elementary stream > permitted. The docs only say "H264 video elementary stream with start > codes.". Admittedly timestamps are nearly meaningless if you feed in > unframed data, but could potentially be interpolated. > > What does other hardware support? > > I could handle it either way, but there are some performance tweaks I > can do if I know the data must be framed. Does anyone have any view on this? Thanks Dave
Re: [PATCH 2/2] media: i2c: mt9t112: Add device tree support
Hi Sakari, On Mon, May 07, 2018 at 12:32:19PM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Wed, Apr 25, 2018 at 01:00:14PM +0200, Jacopo Mondi wrote: [snip] > > static int mt9t112_probe(struct i2c_client *client, > > const struct i2c_device_id *did) > > { > > struct mt9t112_priv *priv; > > int ret; > > > > - if (!client->dev.platform_data) { > > + if (!client->dev.of_node && !client->dev.platform_data) { > > dev_err(&client->dev, "mt9t112: missing platform data!\n"); > > return -EINVAL; > > } > > @@ -1081,23 +1118,39 @@ static int mt9t112_probe(struct i2c_client *client, > > if (!priv) > > return -ENOMEM; > > > > - priv->info = client->dev.platform_data; > > priv->init_done = false; > > - > > - v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops); > > - > > - priv->clk = devm_clk_get(&client->dev, "extclk"); > > - if (PTR_ERR(priv->clk) == -ENOENT) { > > + priv->dev = &client->dev; > > + > > + if (client->dev.platform_data) { > > + priv->info = client->dev.platform_data; > > + > > + priv->clk = devm_clk_get(&client->dev, "extclk"); > > extclk needs to be documented in DT binding documentation. > > > + if (PTR_ERR(priv->clk) == -ENOENT) { > > + priv->clk = NULL; > > + } else if (IS_ERR(priv->clk)) { > > + dev_err(&client->dev, > > + "Unable to get clock \"extclk\"\n"); > > + return PTR_ERR(priv->clk); > > + } > > + } else { > > + /* > > +* External clock frequencies != 24MHz are only supported > > +* for non-OF systems. > > +*/ > > Shouldn't you actually set the frequency? Or perhaps even better to check > it, and use assigned-clocks and assigned-clock-rates properties? > I might be confused, but my intention was to use an external clock reference, with a configurable frequency only in the platform data use case. As you can see in this 'else' branch, in OF case, the priv->clk field is null, and all the PLL and clock computations are performed assuming a 24MHz input clock. In my opinion, as the driver when running on OF systems does not get any reference to 'extclk' clock, it should not be documented in bindings. Do you agree? Thanks j > > priv->clk = NULL; > > - } else if (IS_ERR(priv->clk)) { > > - dev_err(&client->dev, "Unable to get clock \"extclk\"\n"); > > - return PTR_ERR(priv->clk); > > + priv->info = &mt9t112_default_pdata_24MHz; > > + > > + ret = mt9t112_parse_dt(priv); > > + if (ret) > > + return ret; > > } > > > > - priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "standby", > > + v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops); > > + > > + priv->standby_gpio = devm_gpiod_get_optional(&client->dev, "powerdown", > > GPIOD_OUT_HIGH); > > if (IS_ERR(priv->standby_gpio)) { > > - dev_err(&client->dev, "Unable to get gpio \"standby\"\n"); > > + dev_err(&client->dev, "Unable to get gpio \"powerdown\"\n"); > > return PTR_ERR(priv->standby_gpio); > > } > > > > @@ -1124,9 +1177,19 @@ static const struct i2c_device_id mt9t112_id[] = { > > }; > > MODULE_DEVICE_TABLE(i2c, mt9t112_id); > > > > +#if IS_ENABLED(CONFIG_OF) > > +static const struct of_device_id mt9t112_of_match[] = { > > + { .compatible = "micron,mt9t111", }, > > + { .compatible = "micron,mt9t112", }, > > + { /* sentinel */ }, > > +}; > > +MODULE_DEVICE_TABLE(of, mt9t112_of_match); > > +#endif > > + > > static struct i2c_driver mt9t112_i2c_driver = { > > .driver = { > > .name = "mt9t112", > > + .of_match_table = of_match_ptr(mt9t112_of_match), > > No need to use of_match_ptr(). > > > }, > > .probe= mt9t112_probe, > > .remove = mt9t112_remove, > > -- > Sakari Ailus > e-mail: sakari.ai...@iki.fi signature.asc Description: PGP signature
Re: [PATCH v15 1/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
Hi Niklas, thanks for re-sending On Sun, May 13, 2018 at 09:19:16PM +0200, Niklas Söderlund wrote: > Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers > are located between the video sources (CSI-2 transmitters) and the video > grabbers (VIN) on Gen3 of Renesas R-Car SoC. > > Each CSI-2 device is connected to more than one VIN device which > simultaneously can receive video from the same CSI-2 device. Each VIN > device can also be connected to more than one CSI-2 device. The routing > of which links are used is controlled by the VIN devices. There are only > a few possible routes which are set by hardware limitations, which are > different for each SoC in the Gen3 family. > > Signed-off-by: Niklas Söderlund > Acked-by: Rob Herring > Acked-by: Sakari Ailus > Reviewed-by: Laurent Pinchart > > --- > > * Changes since v14. > - Added compatible string for R8A77965 and R8A77970. > - s/Port 0/port@0/ > - s/Port 1/port@1/ > - s/Endpoint 0/endpoint@0/ > > * Changes since v13 > - Add Laurent's tag. > --- > .../bindings/media/renesas,rcar-csi2.txt | 101 ++ > MAINTAINERS | 1 + > 2 files changed, 102 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > > diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > new file mode 100644 > index ..2d385b65b275bc58 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > @@ -0,0 +1,101 @@ > +Renesas R-Car MIPI CSI-2 > + > + > +The R-Car CSI-2 receiver device provides MIPI CSI-2 capabilities for the > +Renesas R-Car family of devices. It is used in conjunction with the > +R-Car VIN module, which provides the video capture capabilities. > + > +Mandatory properties > + > + - compatible: Must be one or more of the following > + - "renesas,r8a7795-csi2" for the R8A7795 device. > + - "renesas,r8a7796-csi2" for the R8A7796 device. > + - "renesas,r8a77965-csi2" for the R8A77965 device. > + - "renesas,r8a77970-csi2" for the R8A77970 device. > + > + - reg: the register base and size for the device registers > + - interrupts: the interrupt for the device > + - clocks: reference to the parent clock > + > +The device node shall contain two 'port' child nodes according to the s/child nodes according/child nodes modeled accordingly/ but don't fully trust my English language skills here. > +bindings defined in Documentation/devicetree/bindings/media/ > +video-interfaces.txt. port@0 shall connect to the CSI-2 source. port@1 > +shall connect to all the R-Car VIN modules that have a hardware > +connection to the CSI-2 receiver. > + > +- port@0- Video source (mandatory) > + - endpoint@0 - sub-node describing the endpoint that is the video source > + > +- port@1 - VIN instances (optional) > + - One endpoint sub-node for every R-Car VIN instance which is connected > + to the R-Car CSI-2 receiver. > + As commented on v14, I feel like there are some restrictions on the accepted values for some endpoint properties in the hardware module, and those should be described here. Eg. the clock lane shall be fixed in position 0 (that's more an HW design choice, I agree), and the number of supported data lanes is either 1, 2 or 4 and 3 is not supported. Apart from that, which I know you do not totally agree on and you're free to ignore. Reviewed-by Jacopo Mondi Thanks j > +Example: > + > + csi20: csi2@fea8 { > + compatible = "renesas,r8a7796-csi2"; > + reg = <0 0xfea8 0 0x1>; > + interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 714>; > + power-domains = <&sysc R8A7796_PD_ALWAYS_ON>; > + resets = <&cpg 714>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + reg = <0>; > + > + csi20_in: endpoint@0 { > + reg = <0>; > + clock-lanes = <0>; > + data-lanes = <1>; > + remote-endpoint = <&adv7482_txb>; > + }; > + }; > + > + port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + reg = <1>; > + > + csi20vin0: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&vin0csi20>; > +
Re: [PATCH v15 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
Hi Niklas, thanks for the patch On Sun, May 13, 2018 at 09:19:17PM +0200, Niklas Söderlund wrote: > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver > supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are > connected between the video sources and the video grabbers (VIN). > > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP. > > Signed-off-by: Niklas Söderlund > > --- > > * Changes since v14 > - Data sheet update changed init sequence for PHY forcing a restructure > of the driver. The restructure was so big I felt compel to drop all > review tags :-( > - The change was that the Renesas H3 procedure was aligned with other > SoC in the Gen3 family procedure. I had kept the rework as separate > patches and was planing to post once original driver with H3 and M3-W > support where merged. As review tags are dropped I chosen to squash > those patches into 2/2. > - Add support for Gen3 V3M. > - Add support for Gen3 M3-N. > - Set PHTC_TESTCLR when stopping the PHY. > - Revert back to the v12 and earlier phypll calculation as it turns out > it was correct after all. > > * Changes since v13 > - Change return rcar_csi2_formats + i to return &rcar_csi2_formats[i]. > - Add define for PHCLM_STOPSTATECKL. > - Update spelling in comments. > - Update calculation in rcar_csi2_calc_phypll() according to > https://linuxtv.org/downloads/v4l-dvb-apis/kapi/csi2.html. The one > before v14 did not take into account that 2 bits per sample is > transmitted. > - Use Geert's suggestion of (1 << priv->lanes) - 1 instead of switch > statement to set correct number of lanes to enable. > - Change hex constants in hsfreqrange_m3w_h3es1[] to lower case to match > style of rest of file. > - Switch to %u instead of 0x%x when printing bus type. > - Switch to %u instead of %d for priv->lanes which is unsigned. > - Add MEDIA_BUS_FMT_YUYV8_1X16 to the list of supported formats in > rcar_csi2_formats[]. > - Fixed bps for MEDIA_BUS_FMT_YUYV10_2X10 to 20 and not 16. > - Set INTSTATE after PL-11 is confirmed to match flow chart in > datasheet. > - Change priv->notifier.subdevs == NULL to !priv->notifier.subdevs. > - Add Maxime's and laurent's tags. > --- > drivers/media/platform/rcar-vin/Kconfig | 12 + > drivers/media/platform/rcar-vin/Makefile|1 + > drivers/media/platform/rcar-vin/rcar-csi2.c | 1101 +++ > 3 files changed, 1114 insertions(+) > create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c > > diff --git a/drivers/media/platform/rcar-vin/Kconfig > b/drivers/media/platform/rcar-vin/Kconfig > index 8fa7ee468c63afb9..d5835da6d4100d87 100644 > --- a/drivers/media/platform/rcar-vin/Kconfig > +++ b/drivers/media/platform/rcar-vin/Kconfig > @@ -1,3 +1,15 @@ > +config VIDEO_RCAR_CSI2 > + tristate "R-Car MIPI CSI-2 Receiver" > + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF > + depends on ARCH_RENESAS || COMPILE_TEST > + select V4L2_FWNODE > + help > + Support for Renesas R-Car MIPI CSI-2 receiver. > + Supports R-Car Gen3 SoCs. > + > + To compile this driver as a module, choose M here: the > + module will be called rcar-csi2. > + > config VIDEO_RCAR_VIN > tristate "R-Car Video Input (VIN) Driver" > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA && > MEDIA_CONTROLLER > diff --git a/drivers/media/platform/rcar-vin/Makefile > b/drivers/media/platform/rcar-vin/Makefile > index 48c5632c21dc060b..5ab803d3e7c1aa57 100644 > --- a/drivers/media/platform/rcar-vin/Makefile > +++ b/drivers/media/platform/rcar-vin/Makefile > @@ -1,3 +1,4 @@ > rcar-vin-objs = rcar-core.o rcar-dma.o rcar-v4l2.o > > +obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o > obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar-vin.o > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c > b/drivers/media/platform/rcar-vin/rcar-csi2.c > new file mode 100644 > index ..b19374f1516464dc > --- /dev/null > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -0,0 +1,1101 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for Renesas R-Car MIPI CSI-2 Receiver > + * > + * Copyright (C) 2018 Renesas Electronics Corp. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +struct rcar_csi2; > + > +/* Register offsets and bits */ > + > +/* Control Timing Select */ > +#define TREF_REG 0x00 > +#define TREF_TREFBIT(0) > + > +/* Software Reset */ > +#define SRST_REG 0x04 > +#define SRST_SRSTBIT(0) > + > +/* PHY Operation Control */ > +#define PHYCNT_REG 0x08 > +#define PHYCNT_SHUTDOWNZ BIT(17) > +#define PHYCNT_RSTZ BIT(16) > +#define PHYCNT_ENABLECLK BIT(4) > +#define PHYCNT_ENABLE_3
[PATCH 3/7] s5p-mfc: fix two sparse warnings
From: Hans Verkuil media-git/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c: In function 'vidioc_querycap': media-git/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c:1317:2: warning: 'strncpy' output may be truncated copying 31 bytes from a string of length 31 [-Wstringop-truncation] strncpy(cap->card, dev->vfd_enc->name, sizeof(cap->card) - 1); ^ media-git/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c: In function 'vidioc_querycap': media-git/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c:275:2: warning: 'strncpy' output may be truncated copying 31 bytes from a string of length 31 [-Wstringop-truncation] strncpy(cap->card, dev->vfd_dec->name, sizeof(cap->card) - 1); ^ Signed-off-by: Hans Verkuil Cc: Sylwester Nawrocki --- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 4 ++-- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c index 5cf4d9921264..6a3cc4f86c5d 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c @@ -271,8 +271,8 @@ static int vidioc_querycap(struct file *file, void *priv, { struct s5p_mfc_dev *dev = video_drvdata(file); - strncpy(cap->driver, S5P_MFC_NAME, sizeof(cap->driver) - 1); - strncpy(cap->card, dev->vfd_dec->name, sizeof(cap->card) - 1); + strlcpy(cap->driver, S5P_MFC_NAME, sizeof(cap->driver)); + strlcpy(cap->card, dev->vfd_dec->name, sizeof(cap->card)); snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", dev_name(&dev->plat_dev->dev)); /* diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c index 5c0462ca9993..570f391f2cfd 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c @@ -1313,8 +1313,8 @@ static int vidioc_querycap(struct file *file, void *priv, { struct s5p_mfc_dev *dev = video_drvdata(file); - strncpy(cap->driver, S5P_MFC_NAME, sizeof(cap->driver) - 1); - strncpy(cap->card, dev->vfd_enc->name, sizeof(cap->card) - 1); + strlcpy(cap->driver, S5P_MFC_NAME, sizeof(cap->driver)); + strlcpy(cap->card, dev->vfd_enc->name, sizeof(cap->card)); snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", dev_name(&dev->plat_dev->dev)); /* -- 2.17.0
[PATCH 0/7] Fix compiler/sparse warnings
From: Hans Verkuil After upgrading to gcc 8.1 we got new warnings. This series fixes them. There is a remaining warning in atomisp, but that's going to be removed from staging anyway. Regards, Hans Hans Verkuil (7): go7007: fix two sparse warnings zoran: fix compiler warning s5p-mfc: fix two sparse warnings hdpvr: fix compiler warning imx: fix compiler warning renesas-ceu: fix compiler warning soc_camera: fix compiler warning drivers/media/platform/renesas-ceu.c| 2 +- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 4 ++-- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 4 ++-- drivers/media/platform/soc_camera/soc_camera_platform.c | 3 ++- drivers/media/usb/go7007/go7007-fw.c| 3 +++ drivers/media/usb/go7007/go7007-v4l2.c | 2 +- drivers/media/usb/hdpvr/hdpvr-video.c | 2 +- drivers/staging/media/imx/imx-media-capture.c | 4 ++-- drivers/staging/media/zoran/zoran_driver.c | 4 ++-- 9 files changed, 16 insertions(+), 12 deletions(-) -- 2.17.0
[PATCH 2/7] zoran: fix compiler warning
From: Hans Verkuil In file included from media-git/include/linux/bitmap.h:9, from media-git/include/linux/cpumask.h:12, from media-git/arch/x86/include/asm/cpumask.h:5, from media-git/arch/x86/include/asm/msr.h:11, from media-git/arch/x86/include/asm/processor.h:21, from media-git/arch/x86/include/asm/cpufeature.h:5, from media-git/arch/x86/include/asm/thread_info.h:53, from media-git/include/linux/thread_info.h:38, from media-git/arch/x86/include/asm/preempt.h:7, from media-git/include/linux/preempt.h:81, from media-git/include/linux/spinlock.h:51, from media-git/include/linux/seqlock.h:36, from media-git/include/linux/time.h:6, from media-git/include/linux/stat.h:19, from media-git/include/linux/module.h:10, from media-git/drivers/staging/media/zoran/zoran_driver.c:44: In function 'strncpy', inlined from 'zoran_querycap' at media-git/drivers/staging/media/zoran/zoran_driver.c:1512:2: media-git/include/linux/string.h:246:9: warning: '__builtin_strncpy' output may be truncated copying 31 bytes from a string of length 31 [-Wstringop-truncation] return __builtin_strncpy(p, q, size); ^ Signed-off-by: Hans Verkuil --- drivers/staging/media/zoran/zoran_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/zoran/zoran_driver.c b/drivers/staging/media/zoran/zoran_driver.c index 14f9c0e26a1c..d2e13fffbc6b 100644 --- a/drivers/staging/media/zoran/zoran_driver.c +++ b/drivers/staging/media/zoran/zoran_driver.c @@ -1509,8 +1509,8 @@ static int zoran_querycap(struct file *file, void *__fh, struct v4l2_capability struct zoran_fh *fh = __fh; struct zoran *zr = fh->zr; - strncpy(cap->card, ZR_DEVNAME(zr), sizeof(cap->card)-1); - strncpy(cap->driver, "zoran", sizeof(cap->driver)-1); + strlcpy(cap->card, ZR_DEVNAME(zr), sizeof(cap->card)); + strlcpy(cap->driver, "zoran", sizeof(cap->driver)); snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s", pci_name(zr->pci_dev)); cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE | -- 2.17.0
[PATCH 1/7] go7007: fix two sparse warnings
From: Hans Verkuil drivers/media/usb/go7007/go7007-v4l2.c:637:2: warning: 'strncpy' specified bound 32 equals destination size [-Wstringop-truncation] drivers/media/usb/go7007/go7007-fw.c:1507:3: warning: this statement may fall through [-Wimplicit-fallthrough=] Signed-off-by: Hans Verkuil --- drivers/media/usb/go7007/go7007-fw.c | 3 +++ drivers/media/usb/go7007/go7007-v4l2.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/go7007/go7007-fw.c b/drivers/media/usb/go7007/go7007-fw.c index 60bf5f0644d1..87b4fc48ef09 100644 --- a/drivers/media/usb/go7007/go7007-fw.c +++ b/drivers/media/usb/go7007/go7007-fw.c @@ -1514,7 +1514,10 @@ static int do_special(struct go7007 *go, u16 type, __le16 *code, int space, case V4L2_PIX_FMT_MPEG4: return gen_mpeg4hdr_to_package(go, code, space, framelen); + default: + break; } + break; case SPECIAL_BRC_CTRL: return brctrl_to_package(go, code, space, framelen); case SPECIAL_CONFIG: diff --git a/drivers/media/usb/go7007/go7007-v4l2.c b/drivers/media/usb/go7007/go7007-v4l2.c index 98cd57eaf36a..c55c82f70e54 100644 --- a/drivers/media/usb/go7007/go7007-v4l2.c +++ b/drivers/media/usb/go7007/go7007-v4l2.c @@ -634,7 +634,7 @@ static int vidioc_enum_input(struct file *file, void *priv, if (inp->index >= go->board_info->num_inputs) return -EINVAL; - strncpy(inp->name, go->board_info->inputs[inp->index].name, + strlcpy(inp->name, go->board_info->inputs[inp->index].name, sizeof(inp->name)); /* If this board has a tuner, it will be the first input */ -- 2.17.0
[PATCH 7/7] soc_camera: fix compiler warning
From: Hans Verkuil In function 'strncpy', inlined from 'soc_camera_platform_probe' at drivers/media/platform/soc_camera/soc_camera_platform.c:162:2: include/linux/string.h:246:9: warning: '__builtin_strncpy' specified bound 32 equals destination size [-Wstringop-truncation] return __builtin_strncpy(p, q, size); ^ Signed-off-by: Hans Verkuil --- drivers/media/platform/soc_camera/soc_camera_platform.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/soc_camera_platform.c b/drivers/media/platform/soc_camera/soc_camera_platform.c index cb4986b8f798..ce00e90d4e3c 100644 --- a/drivers/media/platform/soc_camera/soc_camera_platform.c +++ b/drivers/media/platform/soc_camera/soc_camera_platform.c @@ -159,7 +159,8 @@ static int soc_camera_platform_probe(struct platform_device *pdev) v4l2_subdev_init(&priv->subdev, &platform_subdev_ops); v4l2_set_subdevdata(&priv->subdev, p); - strncpy(priv->subdev.name, dev_name(&pdev->dev), V4L2_SUBDEV_NAME_SIZE); + strlcpy(priv->subdev.name, dev_name(&pdev->dev), + sizeof(priv->subdev.name)); return v4l2_device_register_subdev(&ici->v4l2_dev, &priv->subdev); } -- 2.17.0
[PATCH 4/7] hdpvr: fix compiler warning
From: Hans Verkuil In function 'strncpy', inlined from 'vidioc_g_audio' at media-git/drivers/media/usb/hdpvr/hdpvr-video.c:876:2: media-git/include/linux/string.h:246:9: warning: '__builtin_strncpy' specified bound 32 equals destination size [-Wstringop-truncation] return __builtin_strncpy(p, q, size); ^ Signed-off-by: Hans Verkuil --- drivers/media/usb/hdpvr/hdpvr-video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index 77c3d331ff31..1b89c77bad66 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -873,7 +873,7 @@ static int vidioc_g_audio(struct file *file, void *private_data, audio->index = dev->options.audio_input; audio->capability = V4L2_AUDCAP_STEREO; - strncpy(audio->name, audio_iname[audio->index], sizeof(audio->name)); + strlcpy(audio->name, audio_iname[audio->index], sizeof(audio->name)); audio->name[sizeof(audio->name) - 1] = '\0'; return 0; } -- 2.17.0
[PATCH 5/7] imx: fix compiler warning
From: Hans Verkuil drivers/staging/media/imx/imx-media-capture.c: In function 'vidioc_querycap': drivers/staging/media/imx/imx-media-capture.c:76:2: warning: 'strncpy' output truncated copying 15 bytes from a string of length 17 [-Wstringop-truncation] strncpy(cap->driver, "imx-media-capture", sizeof(cap->driver) - 1); ^~ Signed-off-by: Hans Verkuil Cc: Steve Longerbeam Cc: Philipp Zabel --- drivers/staging/media/imx/imx-media-capture.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c index 0ccabe04b0e1..4e3fdf8aeef5 100644 --- a/drivers/staging/media/imx/imx-media-capture.c +++ b/drivers/staging/media/imx/imx-media-capture.c @@ -73,8 +73,8 @@ static int vidioc_querycap(struct file *file, void *fh, { struct capture_priv *priv = video_drvdata(file); - strncpy(cap->driver, "imx-media-capture", sizeof(cap->driver) - 1); - strncpy(cap->card, "imx-media-capture", sizeof(cap->card) - 1); + strlcpy(cap->driver, "imx-media-capture", sizeof(cap->driver)); + strlcpy(cap->card, "imx-media-capture", sizeof(cap->card)); snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", priv->src_sd->name); -- 2.17.0
[PATCH 6/7] renesas-ceu: fix compiler warning
From: Hans Verkuil In function 'strncpy', inlined from 'ceu_notify_complete' at drivers/media/platform/renesas-ceu.c:1378:2: include/linux/string.h:246:9: warning: '__builtin_strncpy' output truncated before terminating nul copying 11 bytes from a string of the same length [-Wstringop-truncation] return __builtin_strncpy(p, q, size); ^ Signed-off-by: Hans Verkuil Cc: Jacopo Mondi --- drivers/media/platform/renesas-ceu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c index 4879261857fc..fe4fe944592d 100644 --- a/drivers/media/platform/renesas-ceu.c +++ b/drivers/media/platform/renesas-ceu.c @@ -1375,7 +1375,7 @@ static int ceu_notify_complete(struct v4l2_async_notifier *notifier) return ret; /* Register the video device. */ - strncpy(vdev->name, DRIVER_NAME, strlen(DRIVER_NAME)); + strlcpy(vdev->name, DRIVER_NAME, sizeof(vdev->name)); vdev->v4l2_dev = v4l2_dev; vdev->lock = &ceudev->mlock; vdev->queue = &ceudev->vb2_vq; -- 2.17.0
Re: [RFC PATCH 1/6] pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2
On 05/14/2018 01:55 PM, Hans Verkuil wrote: > From: Hans Verkuil > > This driver is the only V4L driver that does not set unlocked_ioctl > to video_ioctl2. > > The only thing that pvr2_v4l2_ioctl does besides calling video_ioctl2 > is calling pvr2_hdw_commit_ctl(). Add pvr2_hdw_commit_ctl() calls to > the various ioctls that need this, and we can replace pvr2_v4l2_ioctl > by video_ioctl2. > > Signed-off-by: Hans Verkuil Tested-by: Hans Verkuil
Re: [RFC PATCH 0/6] v4l2 core: push ioctl lock down to ioctl handler
Hi Mike, On 05/14/2018 01:55 PM, Hans Verkuil wrote: > Mike, can you test this patch? I tried to test it but my pvrusb2 > fails in a USB transfer (unrelated to this patch). I'll mail you > separately with the details, since I've no idea what is going on. Never mind. After unplugging the power and plugging it back in it is now working. Not sure what happened, but at least I can test this now, and it looks fine. BTW, v4l2-compliance complains about a lot of things, and I get a lot of sysfs kernel warnings when I unplug the device. Regards, Has
[RFC PATCH 4/6] videobuf2-core: require q->lock
From: Hans Verkuil Refuse to initialize a vb2 queue if there is no lock specified. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index d3f7bb33a54d..3b89ec5e0b2f 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -2002,6 +2002,7 @@ int vb2_core_queue_init(struct vb2_queue *q) if (WARN_ON(!q) || WARN_ON(!q->ops) || WARN_ON(!q->mem_ops) || + WARN_ON(!q->lock) || WARN_ON(!q->type) || WARN_ON(!q->io_modes) || WARN_ON(!q->ops->queue_setup) || -- 2.17.0
[RFC PATCH 1/6] pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2
From: Hans Verkuil This driver is the only V4L driver that does not set unlocked_ioctl to video_ioctl2. The only thing that pvr2_v4l2_ioctl does besides calling video_ioctl2 is calling pvr2_hdw_commit_ctl(). Add pvr2_hdw_commit_ctl() calls to the various ioctls that need this, and we can replace pvr2_v4l2_ioctl by video_ioctl2. Signed-off-by: Hans Verkuil --- drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 83 +--- 1 file changed, 31 insertions(+), 52 deletions(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c index 9fdc57c1658f..e53a80b589a1 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c @@ -159,9 +159,12 @@ static int pvr2_s_std(struct file *file, void *priv, v4l2_std_id std) { struct pvr2_v4l2_fh *fh = file->private_data; struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; + int ret; - return pvr2_ctrl_set_value( + ret = pvr2_ctrl_set_value( pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_STDCUR), std); + pvr2_hdw_commit_ctl(hdw); + return ret; } static int pvr2_querystd(struct file *file, void *priv, v4l2_std_id *std) @@ -251,12 +254,15 @@ static int pvr2_s_input(struct file *file, void *priv, unsigned int inp) { struct pvr2_v4l2_fh *fh = file->private_data; struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; + int ret; if (inp >= fh->input_cnt) return -EINVAL; - return pvr2_ctrl_set_value( + ret = pvr2_ctrl_set_value( pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_INPUT), fh->input_map[inp]); + pvr2_hdw_commit_ctl(hdw); + return ret; } static int pvr2_enumaudio(struct file *file, void *priv, struct v4l2_audio *vin) @@ -315,13 +321,16 @@ static int pvr2_s_tuner(struct file *file, void *priv, const struct v4l2_tuner * { struct pvr2_v4l2_fh *fh = file->private_data; struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; + int ret; if (vt->index != 0) return -EINVAL; - return pvr2_ctrl_set_value( + ret = pvr2_ctrl_set_value( pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_AUDIOMODE), vt->audmode); + pvr2_hdw_commit_ctl(hdw); + return ret; } static int pvr2_s_frequency(struct file *file, void *priv, const struct v4l2_frequency *vf) @@ -353,8 +362,10 @@ static int pvr2_s_frequency(struct file *file, void *priv, const struct v4l2_fre fv = (fv * 125) / 2; else fv = fv * 62500; - return pvr2_ctrl_set_value( + ret = pvr2_ctrl_set_value( pvr2_hdw_get_ctrl_by_id(hdw,PVR2_CID_FREQUENCY),fv); + pvr2_hdw_commit_ctl(hdw); + return ret; } static int pvr2_g_frequency(struct file *file, void *priv, struct v4l2_frequency *vf) @@ -470,6 +481,7 @@ static int pvr2_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format vcp = pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_VRES); pvr2_ctrl_set_value(hcp, vf->fmt.pix.width); pvr2_ctrl_set_value(vcp, vf->fmt.pix.height); + pvr2_hdw_commit_ctl(hdw); return 0; } @@ -597,9 +609,12 @@ static int pvr2_s_ctrl(struct file *file, void *priv, struct v4l2_control *vc) { struct pvr2_v4l2_fh *fh = file->private_data; struct pvr2_hdw *hdw = fh->channel.mc_head->hdw; + int ret; - return pvr2_ctrl_set_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id), + ret = pvr2_ctrl_set_value(pvr2_hdw_get_ctrl_v4l(hdw, vc->id), vc->value); + pvr2_hdw_commit_ctl(hdw); + return ret; } static int pvr2_g_ext_ctrls(struct file *file, void *priv, @@ -658,10 +673,12 @@ static int pvr2_s_ext_ctrls(struct file *file, void *priv, ctrl->value); if (ret) { ctls->error_idx = idx; - return ret; + goto commit; } } - return 0; +commit: + pvr2_hdw_commit_ctl(hdw); + return ret; } static int pvr2_try_ext_ctrls(struct file *file, void *priv, @@ -764,23 +781,23 @@ static int pvr2_s_selection(struct file *file, void *priv, pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_CROPL), sel->r.left); if (ret != 0) - return -EINVAL; + goto commit; ret = pvr2_ctrl_set_value( pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_CROPT), sel->r.top); if (ret != 0) - return -EINVAL; + goto commit; ret = pvr2_ctrl_set_value( pvr2_hdw_get_ctrl_by_id(hdw, PVR2_CID_CROPW), sel->r.width); if (ret != 0) - return -EINVAL; + goto commit; re
[RFC PATCH 2/6] v4l2-core: push taking ioctl mutex down to ioctl handler.
From: Hans Verkuil The ioctl serialization mutex (vdev->lock or q->lock for vb2 queues) was taken at the highest level in v4l2-dev.c. This prevents more fine-grained locking since at that level we cannot examine the ioctl arguments, we can only do that after video_usercopy is called. So push the locking down to __video_do_ioctl() and subdev_do_ioctl_lock(). This also allows us to make a few functions in v4l2-ioctl.c static and video_usercopy() is no longer exported. The locking scheme is not changed by this patch, just pushed down. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-dev.c| 6 -- drivers/media/v4l2-core/v4l2-ioctl.c | 17 ++--- drivers/media/v4l2-core/v4l2-subdev.c | 17 - include/media/v4l2-dev.h | 9 - include/media/v4l2-ioctl.h| 12 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index c4f4357e9ca4..4ffd7d60a901 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -360,14 +360,8 @@ static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) int ret = -ENODEV; if (vdev->fops->unlocked_ioctl) { - struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd); - - if (lock && mutex_lock_interruptible(lock)) - return -ERESTARTSYS; if (video_is_registered(vdev)) ret = vdev->fops->unlocked_ioctl(filp, cmd, arg); - if (lock) - mutex_unlock(lock); } else ret = -ENOTTY; diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index de5d96dbe69e..a12c66ead30d 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -2619,14 +2619,14 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = { }; #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) -bool v4l2_is_known_ioctl(unsigned int cmd) +static bool v4l2_is_known_ioctl(unsigned int cmd) { if (_IOC_NR(cmd) >= V4L2_IOCTLS) return false; return v4l2_ioctls[_IOC_NR(cmd)].ioctl == cmd; } -struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned cmd) +static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned cmd) { if (_IOC_NR(cmd) >= V4L2_IOCTLS) return vdev->lock; @@ -2679,6 +2679,7 @@ static long __video_do_ioctl(struct file *file, unsigned int cmd, void *arg) { struct video_device *vfd = video_devdata(file); + struct mutex *lock = v4l2_ioctl_get_lock(vfd, cmd); const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops; bool write_only = false; struct v4l2_ioctl_info default_info; @@ -2697,6 +2698,14 @@ static long __video_do_ioctl(struct file *file, if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) vfh = file->private_data; + if (lock && mutex_lock_interruptible(lock)) + return -ERESTARTSYS; + + if (!video_is_registered(vfd)) { + ret = -ENODEV; + goto unlock; + } + if (v4l2_is_known_ioctl(cmd)) { info = &v4l2_ioctls[_IOC_NR(cmd)]; @@ -2752,6 +2761,9 @@ static long __video_do_ioctl(struct file *file, } } +unlock: + if (lock) + mutex_unlock(lock); return ret; } @@ -2941,7 +2953,6 @@ video_usercopy(struct file *file, unsigned int cmd, unsigned long arg, kvfree(mbuf); return err; } -EXPORT_SYMBOL(video_usercopy); long video_ioctl2(struct file *file, unsigned int cmd, unsigned long arg) diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index f9eed938d348..6a7f7f75dfd7 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -502,10 +502,25 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) return 0; } +static long subdev_do_ioctl_lock(struct file *file, unsigned int cmd, void *arg) +{ + struct video_device *vdev = video_devdata(file); + struct mutex *lock = vdev->lock; + long ret = -ENODEV; + + if (lock && mutex_lock_interruptible(lock)) + return -ERESTARTSYS; + if (video_is_registered(vdev)) + ret = subdev_do_ioctl(file, cmd, arg); + if (lock) + mutex_unlock(lock); + return ret; +} + static long subdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { - return video_usercopy(file, cmd, arg, subdev_do_ioctl); + return video_usercopy(file, cmd, arg, subdev_do_ioctl_lock); } #ifdef CONFIG_COMPAT diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h index 73073f6ee48c..eb23f025aa6b 100644 --- a/include/
[RFC PATCH 6/6] v4l2-ioctl.c: assume queue->lock is always set
From: Hans Verkuil vb2_queue now expects a valid lock pointer, so drop the checks for that in v4l2-ioctl.c. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ioctl.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index b871b8fe5105..15bfbc6ce6c0 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -2681,12 +2681,11 @@ static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, struct v4l2_m2m_queue_ctx *ctx = is_output ? &vfh->m2m_ctx->out_q_ctx : &vfh->m2m_ctx->cap_q_ctx; - if (ctx->q.lock) - return ctx->q.lock; + return ctx->q.lock; } #endif - if (vdev->queue && vdev->queue->lock && - (v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE)) + if (vdev->queue && + (v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE)) return vdev->queue->lock; return vdev->lock; } -- 2.17.0
[RFC PATCH 3/6] v4l2-ioctl.c: use correct vb2_queue lock for m2m devices
From: Hans Verkuil For m2m devices the vdev->queue lock was always taken instead of the lock for the specific capture or output queue. Now that we pushed the locking down into __video_do_ioctl() we can pick the correct lock and improve the performance of m2m devices. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ioctl.c | 59 +++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index a12c66ead30d..b871b8fe5105 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -29,6 +29,7 @@ #include #include #include +#include #include @@ -2626,12 +2627,64 @@ static bool v4l2_is_known_ioctl(unsigned int cmd) return v4l2_ioctls[_IOC_NR(cmd)].ioctl == cmd; } -static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, unsigned cmd) +#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV) +static bool v4l2_ioctl_m2m_queue_is_output(unsigned int cmd, void *arg) +{ + switch (cmd) { + case VIDIOC_CREATE_BUFS: { + struct v4l2_create_buffers *cbufs = arg; + + return V4L2_TYPE_IS_OUTPUT(cbufs->format.type); + } + case VIDIOC_REQBUFS: { + struct v4l2_requestbuffers *rbufs = arg; + + return V4L2_TYPE_IS_OUTPUT(rbufs->type); + } + case VIDIOC_QBUF: + case VIDIOC_DQBUF: + case VIDIOC_QUERYBUF: + case VIDIOC_PREPARE_BUF: { + struct v4l2_buffer *buf = arg; + + return V4L2_TYPE_IS_OUTPUT(buf->type); + } + case VIDIOC_EXPBUF: { + struct v4l2_exportbuffer *expbuf = arg; + + return V4L2_TYPE_IS_OUTPUT(expbuf->type); + } + case VIDIOC_STREAMON: + case VIDIOC_STREAMOFF: { + int *type = arg; + + return V4L2_TYPE_IS_OUTPUT(*type); + } + default: + return false; + } +} +#endif + +static struct mutex *v4l2_ioctl_get_lock(struct video_device *vdev, +struct v4l2_fh *vfh, unsigned cmd, +void *arg) { if (_IOC_NR(cmd) >= V4L2_IOCTLS) return vdev->lock; if (test_bit(_IOC_NR(cmd), vdev->disable_locking)) return NULL; +#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV) + if (vfh && vfh->m2m_ctx && + (v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE)) { + bool is_output = v4l2_ioctl_m2m_queue_is_output(cmd, arg); + struct v4l2_m2m_queue_ctx *ctx = is_output ? + &vfh->m2m_ctx->out_q_ctx : &vfh->m2m_ctx->cap_q_ctx; + + if (ctx->q.lock) + return ctx->q.lock; + } +#endif if (vdev->queue && vdev->queue->lock && (v4l2_ioctls[_IOC_NR(cmd)].flags & INFO_FL_QUEUE)) return vdev->queue->lock; @@ -2679,7 +2732,7 @@ static long __video_do_ioctl(struct file *file, unsigned int cmd, void *arg) { struct video_device *vfd = video_devdata(file); - struct mutex *lock = v4l2_ioctl_get_lock(vfd, cmd); + struct mutex *lock; const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops; bool write_only = false; struct v4l2_ioctl_info default_info; @@ -2698,6 +2751,8 @@ static long __video_do_ioctl(struct file *file, if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) vfh = file->private_data; + lock = v4l2_ioctl_get_lock(vfd, vfh, cmd, arg); + if (lock && mutex_lock_interruptible(lock)) return -ERESTARTSYS; -- 2.17.0
[RFC PATCH 5/6] videobuf2: assume q->lock is always set
From: Hans Verkuil Drop checks for q->lock. Drop calls to wait_finish/prepare, just lock/unlock q->lock. Signed-off-by: Hans Verkuil --- .../media/common/videobuf2/videobuf2-core.c | 21 ++- .../media/common/videobuf2/videobuf2-v4l2.c | 27 +-- include/media/videobuf2-core.h| 2 -- 3 files changed, 15 insertions(+), 35 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 3b89ec5e0b2f..8ca279a43549 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -462,8 +462,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) * counters to the kernel log. */ if (q->num_buffers) { - bool unbalanced = q->cnt_start_streaming != q->cnt_stop_streaming || - q->cnt_wait_prepare != q->cnt_wait_finish; + bool unbalanced = q->cnt_start_streaming != q->cnt_stop_streaming; if (unbalanced || debug) { pr_info("counters for queue %p:%s\n", q, @@ -471,12 +470,8 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) pr_info(" setup: %u start_streaming: %u stop_streaming: %u\n", q->cnt_queue_setup, q->cnt_start_streaming, q->cnt_stop_streaming); - pr_info(" wait_prepare: %u wait_finish: %u\n", - q->cnt_wait_prepare, q->cnt_wait_finish); } q->cnt_queue_setup = 0; - q->cnt_wait_prepare = 0; - q->cnt_wait_finish = 0; q->cnt_start_streaming = 0; q->cnt_stop_streaming = 0; } @@ -1484,10 +1479,10 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) /* * We are streaming and blocking, wait for another buffer to -* become ready or for streamoff. Driver's lock is released to +* become ready or for streamoff. The queue's lock is released to * allow streamoff or qbuf to be called while waiting. */ - call_void_qop(q, wait_prepare, q); + mutex_unlock(q->lock); /* * All locks have been released, it is safe to sleep now. @@ -1501,7 +1496,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) * We need to reevaluate both conditions again after reacquiring * the locks or return an error if one occurred. */ - call_void_qop(q, wait_finish, q); + mutex_lock(q->lock); if (ret) { dprintk(1, "sleep was interrupted\n"); return ret; @@ -2528,10 +2523,10 @@ static int vb2_thread(void *data) vb = q->bufs[index++]; prequeue--; } else { - call_void_qop(q, wait_finish, q); + mutex_lock(q->lock); if (!threadio->stop) ret = vb2_core_dqbuf(q, &index, NULL, 0); - call_void_qop(q, wait_prepare, q); + mutex_unlock(q->lock); dprintk(5, "file io: vb2_dqbuf result: %d\n", ret); if (!ret) vb = q->bufs[index]; @@ -2543,12 +2538,12 @@ static int vb2_thread(void *data) if (vb->state != VB2_BUF_STATE_ERROR) if (threadio->fnc(vb, threadio->priv)) break; - call_void_qop(q, wait_finish, q); + mutex_lock(q->lock); if (copy_timestamp) vb->timestamp = ktime_get_ns(); if (!threadio->stop) ret = vb2_core_qbuf(q, vb->index, NULL); - call_void_qop(q, wait_prepare, q); + mutex_unlock(q->lock); if (ret || threadio->stop) break; } diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 886a2d8d5c6c..7d2172468f72 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -852,9 +852,8 @@ EXPORT_SYMBOL_GPL(_vb2_fop_release); int vb2_fop_release(struct file *file) { struct video_device *vdev = video_devdata(file); - struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock; - return _vb2_fop_release(file, lock); + return _vb2_fop_release(file, vdev->queue->lock); } EXPORT_SYMBOL_GPL(vb2_fop_release); @@ -862,12 +861,11 @@ ssize_t vb2_fop_wr
[RFC PATCH 0/6] v4l2 core: push ioctl lock down to ioctl handler
From: Hans Verkuil While working on the DMA fence API and the Request API it became clear that the core ioctl scheme was done at a too-high level. Being able to actually look at the struct passed as the ioctl argument would help a lot in decide what lock(s) to take. This patch series pushes the lock down into v4l2-ioctl.c, after video_usercopy() was called. The first patch is for the only driver that does not set unlocked_ioctl to video_ioctl2: pvrusb2. It actually does call it in its own unlocked_ioctl function. Mike, can you test this patch? I tried to test it but my pvrusb2 fails in a USB transfer (unrelated to this patch). I'll mail you separately with the details, since I've no idea what is going on. The second patch pushes the lock down. The third patch adds support for mem2mem devices by selecting the correct queue lock (capture vs output): this was never possible before. Note: in practice it appears that all m2m devices use the same lock for both capture and output queues. Perhaps this should be standardized? The final three patches require that queue->lock is always set. There are some drivers that do not set this (and Ezequiel will look at that and provide patches that will have to go in between patch 3 and 4 of this RFC series), but without that you will have performance issues with a blocking DQBUF (it will never release the core ioctl serialization lock while waiting for a new frame). I have added a test for this to v4l2-compliance. We never tested this before. Another note: the gspca vb2 conversion series I posted yesterday also remove the v4l2_disable_ioctl_locking() function, so that cleans up another little locking-related dark corner in the core. Regards, Hans Hans Verkuil (6): pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2 v4l2-core: push taking ioctl mutex down to ioctl handler. v4l2-ioctl.c: use correct vb2_queue lock for m2m devices videobuf2-core: require q->lock videobuf2: assume q->lock is always set v4l2-ioctl.c: assume queue->lock is always set .../media/common/videobuf2/videobuf2-core.c | 22 ++--- .../media/common/videobuf2/videobuf2-v4l2.c | 27 ++ drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 83 +++ drivers/media/v4l2-core/v4l2-dev.c| 6 -- drivers/media/v4l2-core/v4l2-ioctl.c | 75 +++-- drivers/media/v4l2-core/v4l2-subdev.c | 17 +++- include/media/v4l2-dev.h | 9 -- include/media/v4l2-ioctl.h| 12 --- include/media/videobuf2-core.h| 2 - 9 files changed, 133 insertions(+), 120 deletions(-) -- 2.17.0
[PATCH v2 2/5] media: docs: clarify relationship between crop and selection APIs
Having two somewhat similar and largely overlapping APIs is confusing, especially since the older one appears in the docs before the newer and most featureful counterpart. Clarify all of this in several ways: - swap the two sections - give a name to the two APIs in the section names - add a note at the beginning of the CROP API section - update note about VIDIOC_CROPCAP Also remove a note that is incorrect (correct wording is in vidioc-cropcap.rst). Signed-off-by: Luca Ceresoli Based on info from: Hans Verkuil Cc: Hans Verkuil --- Changed v1 -> v2: - Don't remove the note about VIDIOC_CROPCAP being mandatory, update it (Hans) --- Documentation/media/uapi/v4l/common.rst| 2 +- Documentation/media/uapi/v4l/crop.rst | 22 +++--- Documentation/media/uapi/v4l/selection-api-005.rst | 2 ++ Documentation/media/uapi/v4l/selection-api.rst | 4 ++-- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/Documentation/media/uapi/v4l/common.rst b/Documentation/media/uapi/v4l/common.rst index 13f2ed3fc5a6..5f93e71122ef 100644 --- a/Documentation/media/uapi/v4l/common.rst +++ b/Documentation/media/uapi/v4l/common.rst @@ -41,6 +41,6 @@ applicable to all devices. extended-controls format planar-apis -crop selection-api +crop streaming-par diff --git a/Documentation/media/uapi/v4l/crop.rst b/Documentation/media/uapi/v4l/crop.rst index 182565b9ace4..45e8a895a320 100644 --- a/Documentation/media/uapi/v4l/crop.rst +++ b/Documentation/media/uapi/v4l/crop.rst @@ -2,9 +2,18 @@ .. _crop: -* -Image Cropping, Insertion and Scaling -* +* +Image Cropping, Insertion and Scaling -- the CROP API +* + +.. note:: + + The CROP API is mostly superseded by the newer :ref:`SELECTION API + `. The new API should be preferred in most cases, + with the exception of pixel aspect ratio detection, which is + implemented by :ref:`VIDIOC_CROPCAP ` and has no + equivalent in the SELECTION API. See :ref:`selection-vs-crop` for a + comparison of the two APIs. Some video capture devices can sample a subsection of the picture and shrink or enlarge it to an image of arbitrary size. We call these @@ -42,10 +51,9 @@ where applicable) will be fixed in this case. .. note:: - All capture and output devices must support the - :ref:`VIDIOC_CROPCAP ` ioctl such that applications - can determine if scaling takes place. - + All capture and output devices that support the CROP or SELECTION + API will also support the :ref:`VIDIOC_CROPCAP ` + ioctl. Cropping Structures === diff --git a/Documentation/media/uapi/v4l/selection-api-005.rst b/Documentation/media/uapi/v4l/selection-api-005.rst index 5b47a28ac6d7..2ad30a49184f 100644 --- a/Documentation/media/uapi/v4l/selection-api-005.rst +++ b/Documentation/media/uapi/v4l/selection-api-005.rst @@ -1,5 +1,7 @@ .. -*- coding: utf-8; mode: rst -*- +.. _selection-vs-crop: + Comparison with old cropping API diff --git a/Documentation/media/uapi/v4l/selection-api.rst b/Documentation/media/uapi/v4l/selection-api.rst index 81ea52d785b9..e4e623824b30 100644 --- a/Documentation/media/uapi/v4l/selection-api.rst +++ b/Documentation/media/uapi/v4l/selection-api.rst @@ -2,8 +2,8 @@ .. _selection-api: -API for cropping, composing and scaling -=== +Cropping, composing and scaling -- the SELECTION API + .. toctree:: -- 2.7.4
[PATCH v2 3/5] media: docs: selection: rename files to something meaningful
These files have an automatically-generated numbering. Rename them with a name that suggests their meaning. Reported-by: Hans Verkuil Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- Changed v1 -> v2: - fix commit message typos (Hans) --- .../{selection-api-004.rst => selection-api-configuration.rst} | 0 .../v4l/{selection-api-006.rst => selection-api-examples.rst} | 0 .../v4l/{selection-api-002.rst => selection-api-intro.rst} | 0 .../v4l/{selection-api-003.rst => selection-api-targets.rst} | 0 .../{selection-api-005.rst => selection-api-vs-crop-api.rst} | 0 Documentation/media/uapi/v4l/selection-api.rst | 10 +- 6 files changed, 5 insertions(+), 5 deletions(-) rename Documentation/media/uapi/v4l/{selection-api-004.rst => selection-api-configuration.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-006.rst => selection-api-examples.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-002.rst => selection-api-intro.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-003.rst => selection-api-targets.rst} (100%) rename Documentation/media/uapi/v4l/{selection-api-005.rst => selection-api-vs-crop-api.rst} (100%) diff --git a/Documentation/media/uapi/v4l/selection-api-004.rst b/Documentation/media/uapi/v4l/selection-api-configuration.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-004.rst rename to Documentation/media/uapi/v4l/selection-api-configuration.rst diff --git a/Documentation/media/uapi/v4l/selection-api-006.rst b/Documentation/media/uapi/v4l/selection-api-examples.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-006.rst rename to Documentation/media/uapi/v4l/selection-api-examples.rst diff --git a/Documentation/media/uapi/v4l/selection-api-002.rst b/Documentation/media/uapi/v4l/selection-api-intro.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-002.rst rename to Documentation/media/uapi/v4l/selection-api-intro.rst diff --git a/Documentation/media/uapi/v4l/selection-api-003.rst b/Documentation/media/uapi/v4l/selection-api-targets.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-003.rst rename to Documentation/media/uapi/v4l/selection-api-targets.rst diff --git a/Documentation/media/uapi/v4l/selection-api-005.rst b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst similarity index 100% rename from Documentation/media/uapi/v4l/selection-api-005.rst rename to Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst diff --git a/Documentation/media/uapi/v4l/selection-api.rst b/Documentation/media/uapi/v4l/selection-api.rst index e4e623824b30..390233f704a3 100644 --- a/Documentation/media/uapi/v4l/selection-api.rst +++ b/Documentation/media/uapi/v4l/selection-api.rst @@ -9,8 +9,8 @@ Cropping, composing and scaling -- the SELECTION API .. toctree:: :maxdepth: 1 -selection-api-002 -selection-api-003 -selection-api-004 -selection-api-005 -selection-api-006 +selection-api-intro.rst +selection-api-targets.rst +selection-api-configuration.rst +selection-api-vs-crop-api.rst +selection-api-examples.rst -- 2.7.4
[PATCH v2 5/5] media: docs: selection: fix misleading sentence about the CROP API
The API limitation described here is about the CROP API, not about the entire V4L2. Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- Changed v1 -> v2: nothing. --- Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst index ba1064a244a0..e7455fb1e572 100644 --- a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst +++ b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst @@ -15,7 +15,7 @@ because the described operation is actually the composing. The selection API makes a clear distinction between composing and cropping operations by setting the appropriate targets. -The V4L2 API lacks any support for composing to and cropping from an +The CROP API lacks any support for composing to and cropping from an image inside a memory buffer. The application could configure a capture device to fill only a part of an image by abusing V4L2 API. Cropping a smaller image from a larger one is achieved by setting -- 2.7.4
[PATCH v2 1/5] media: docs: selection: fix typos
Fix typos in the selection documentation: - over -> cover - BONDS -> BOUNDS Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- Changed v1 -> v2: - add a commit message (Hans) --- Documentation/media/uapi/v4l/selection-api-004.rst | 2 +- Documentation/media/uapi/v4l/selection.svg | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/media/uapi/v4l/selection-api-004.rst b/Documentation/media/uapi/v4l/selection-api-004.rst index d782cd5b2117..0a4ddc2d71db 100644 --- a/Documentation/media/uapi/v4l/selection-api-004.rst +++ b/Documentation/media/uapi/v4l/selection-api-004.rst @@ -41,7 +41,7 @@ The driver may further adjust the requested size and/or position according to hardware limitations. Each capture device has a default source rectangle, given by the -``V4L2_SEL_TGT_CROP_DEFAULT`` target. This rectangle shall over what the +``V4L2_SEL_TGT_CROP_DEFAULT`` target. This rectangle shall cover what the driver writer considers the complete picture. Drivers shall set the active crop rectangle to the default when the driver is first loaded, but not later. diff --git a/Documentation/media/uapi/v4l/selection.svg b/Documentation/media/uapi/v4l/selection.svg index a93e3b59786d..911062bd2844 100644 --- a/Documentation/media/uapi/v4l/selection.svg +++ b/Documentation/media/uapi/v4l/selection.svg @@ -1128,11 +1128,11 @@ - COMPOSE_BONDS + COMPOSE_BOUNDS -CROP_BONDS +CROP_BOUNDS overscan area -- 2.7.4
[PATCH v2 4/5] media: docs: selection: improve formatting
Split section "Comparison with old cropping API" in paragraphs for easier reading and improve visible links text. Cc: Hans Verkuil Signed-off-by: Luca Ceresoli --- Changed v1 -> v2: nothing. --- .../media/uapi/v4l/selection-api-vs-crop-api.rst | 55 -- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst index 2ad30a49184f..ba1064a244a0 100644 --- a/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst +++ b/Documentation/media/uapi/v4l/selection-api-vs-crop-api.rst @@ -6,31 +6,34 @@ Comparison with old cropping API -The selection API was introduced to cope with deficiencies of previous -:ref:`API `, that was designed to control simple capture -devices. Later the cropping API was adopted by video output drivers. The -ioctls are used to select a part of the display were the video signal is -inserted. It should be considered as an API abuse because the described -operation is actually the composing. The selection API makes a clear -distinction between composing and cropping operations by setting the -appropriate targets. The V4L2 API lacks any support for composing to and -cropping from an image inside a memory buffer. The application could -configure a capture device to fill only a part of an image by abusing -V4L2 API. Cropping a smaller image from a larger one is achieved by -setting the field ``bytesperline`` at struct -:c:type:`v4l2_pix_format`. -Introducing an image offsets could be done by modifying field ``m_userptr`` -at struct -:c:type:`v4l2_buffer` before calling -:ref:`VIDIOC_QBUF`. Those operations should be avoided because they are not -portable (endianness), and do not work for macroblock and Bayer formats -and mmap buffers. The selection API deals with configuration of buffer +The selection API was introduced to cope with deficiencies of the +older :ref:`CROP API `, that was designed to control simple +capture devices. Later the cropping API was adopted by video output +drivers. The ioctls are used to select a part of the display were the +video signal is inserted. It should be considered as an API abuse +because the described operation is actually the composing. The +selection API makes a clear distinction between composing and cropping +operations by setting the appropriate targets. + +The V4L2 API lacks any support for composing to and cropping from an +image inside a memory buffer. The application could configure a +capture device to fill only a part of an image by abusing V4L2 +API. Cropping a smaller image from a larger one is achieved by setting +the field ``bytesperline`` at struct :c:type:`v4l2_pix_format`. +Introducing an image offsets could be done by modifying field +``m_userptr`` at struct :c:type:`v4l2_buffer` before calling +:ref:`VIDIOC_QBUF `. Those operations should be avoided +because they are not portable (endianness), and do not work for +macroblock and Bayer formats and mmap buffers. + +The selection API deals with configuration of buffer cropping/composing in a clear, intuitive and portable way. Next, with the selection API the concepts of the padded target and constraints -flags are introduced. Finally, struct :c:type:`v4l2_crop` -and struct :c:type:`v4l2_cropcap` have no reserved -fields. Therefore there is no way to extend their functionality. The new -struct :c:type:`v4l2_selection` provides a lot of place -for future extensions. Driver developers are encouraged to implement -only selection API. The former cropping API would be simulated using the -new one. +flags are introduced. Finally, struct :c:type:`v4l2_crop` and struct +:c:type:`v4l2_cropcap` have no reserved fields. Therefore there is no +way to extend their functionality. The new struct +:c:type:`v4l2_selection` provides a lot of place for future +extensions. + +Driver developers are encouraged to implement only selection API. The +former cropping API would be simulated using the new one. -- 2.7.4
Re: [PATCH 1/5] media: docs: selection: fix typos
Hi Hans, On 13/05/2018 11:19, Hans Verkuil wrote: > Hi Luca, > > My apologies for the long delay in reviewing this. > > It all looks very good and if you can post a v2 with these small issues > fixed, then I'll merge it for 4.18. On its way! Thanks, -- Luca
Re: [PATCH 2/5] media: docs: clarify relationship between crop and selection APIs
Hi Hans, thanks for the review. On 13/05/2018 11:12, Hans Verkuil wrote: > On 04/03/2018 11:15 PM, Luca Ceresoli wrote: >> Having two somewhat similar and largely overlapping APIs is confusing, >> especially since the older one appears in the docs before the newer >> and most featureful counterpart. >> >> Clarify all of this in several ways: >> - swap the two sections >> - give a name to the two APIs in the section names >> - add a note at the beginning of the CROP API section >> >> Also remove a note that is incorrect (correct wording is in >> vidioc-cropcap.rst). >> >> Signed-off-by: Luca Ceresoli >> Based on info from: Hans Verkuil >> Cc: Hans Verkuil >> --- >> Documentation/media/uapi/v4l/common.rst| 2 +- >> Documentation/media/uapi/v4l/crop.rst | 21 >> - >> Documentation/media/uapi/v4l/selection-api-005.rst | 2 ++ >> Documentation/media/uapi/v4l/selection-api.rst | 4 ++-- >> 4 files changed, 17 insertions(+), 12 deletions(-) >> >> diff --git a/Documentation/media/uapi/v4l/common.rst >> b/Documentation/media/uapi/v4l/common.rst >> index 13f2ed3fc5a6..5f93e71122ef 100644 >> --- a/Documentation/media/uapi/v4l/common.rst >> +++ b/Documentation/media/uapi/v4l/common.rst >> @@ -41,6 +41,6 @@ applicable to all devices. >> extended-controls >> format >> planar-apis >> -crop >> selection-api >> +crop >> streaming-par >> diff --git a/Documentation/media/uapi/v4l/crop.rst >> b/Documentation/media/uapi/v4l/crop.rst >> index 182565b9ace4..83fa16eb347e 100644 >> --- a/Documentation/media/uapi/v4l/crop.rst >> +++ b/Documentation/media/uapi/v4l/crop.rst >> @@ -2,9 +2,18 @@ >> >> .. _crop: >> >> -* >> -Image Cropping, Insertion and Scaling >> -* >> +* >> +Image Cropping, Insertion and Scaling -- the CROP API >> +* >> + >> +.. note:: >> + >> + The CROP API is mostly superseded by the newer :ref:`SELECTION API >> + `. The new API should be preferred in most cases, >> + with the exception of pixel aspect ratio detection, which is >> + implemented by :ref:`VIDIOC_CROPCAP ` and has no >> + equivalent in the SELECTION API. See :ref:`selection-vs-crop` for a >> + comparison of the two APIs. >> >> Some video capture devices can sample a subsection of the picture and >> shrink or enlarge it to an image of arbitrary size. We call these >> @@ -40,12 +49,6 @@ support scaling or the :ref:`VIDIOC_G_CROP >> ` and >> :ref:`VIDIOC_S_CROP ` ioctls. Their size (and position >> where applicable) will be fixed in this case. >> >> -.. note:: >> - >> - All capture and output devices must support the >> - :ref:`VIDIOC_CROPCAP ` ioctl such that applications >> - can determine if scaling takes place. > > This note should be rewritten, not deleted: > > All capture and output devices that support the CROP or SELECTION API > will also support the :ref:`VIDIOC_CROPCAP ` ioctl. I don't remember exactly the reationale for this removal, perhaps it's that I added a note above with similar content. But reading that again now I realize the added not does not clearly state "VIDIOC_CROPCAP is mandatory". Thus I added the updated note to v2 as you suggested. Bye, -- Luca
Re: [PATCH v5 08/14] media: ov772x: support device tree probing
On Mon, May 14, 2018 at 11:16:42AM +0200, jacopo mondi wrote: > Hi Sakari, > > On Mon, May 07, 2018 at 12:26:41PM +0300, Sakari Ailus wrote: > > Dear Mita-san, > > > > On Sun, May 06, 2018 at 11:19:23PM +0900, Akinobu Mita wrote: > > > The ov772x driver currently only supports legacy platform data probe. > > > This change enables device tree probing. > > > > > > Note that the platform data probe can select auto or manual edge control > > > mode, but the device tree probling can only select auto edge control mode > > > for now. > > > > > > Cc: Jacopo Mondi > > > Cc: Laurent Pinchart > > > Cc: Hans Verkuil > > > Cc: Sakari Ailus > > > Cc: Mauro Carvalho Chehab > > > Reviewed-by: Jacopo Mondi > > > Signed-off-by: Akinobu Mita > > > --- > > > * v5 > > > - Remove unnecessary space > > > > > > drivers/media/i2c/ov772x.c | 64 > > > -- > > > 1 file changed, 45 insertions(+), 19 deletions(-) > > > > > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > > > index f939e28..2b02411 100644 > > > --- a/drivers/media/i2c/ov772x.c > > > +++ b/drivers/media/i2c/ov772x.c > > > @@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > > > case V4L2_CID_VFLIP: > > > val = ctrl->val ? VFLIP_IMG : 0x00; > > > priv->flag_vflip = ctrl->val; > > > - if (priv->info->flags & OV772X_FLAG_VFLIP) > > > + if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP)) > > > val ^= VFLIP_IMG; > > > return ov772x_mask_set(client, COM3, VFLIP_IMG, val); > > > case V4L2_CID_HFLIP: > > > val = ctrl->val ? HFLIP_IMG : 0x00; > > > priv->flag_hflip = ctrl->val; > > > - if (priv->info->flags & OV772X_FLAG_HFLIP) > > > + if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP)) > > > val ^= HFLIP_IMG; > > > return ov772x_mask_set(client, COM3, HFLIP_IMG, val); > > > case V4L2_CID_BAND_STOP_FILTER: > > > @@ -914,19 +914,14 @@ static void ov772x_select_params(const struct > > > v4l2_mbus_framefmt *mf, > > > *win = ov772x_select_win(mf->width, mf->height); > > > } > > > > > > -static int ov772x_set_params(struct ov772x_priv *priv, > > > - const struct ov772x_color_format *cfmt, > > > - const struct ov772x_win_size *win) > > > +static int ov772x_edgectrl(struct ov772x_priv *priv) > > > { > > > struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > > > - struct v4l2_fract tpf; > > > int ret; > > > - u8 val; > > > > > > - /* Reset hardware. */ > > > - ov772x_reset(client); > > > + if (!priv->info) > > > + return 0; > > > > > > - /* Edge Ctrl. */ > > > if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) { > > > /* > > >* Manual Edge Control Mode. > > > @@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv > > > *priv, > > > > > > ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00); > > > if (ret < 0) > > > - goto ov772x_set_fmt_error; > > > + return ret; > > > > > > ret = ov772x_mask_set(client, > > > EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK, > > > priv->info->edgectrl.threshold); > > > if (ret < 0) > > > - goto ov772x_set_fmt_error; > > > + return ret; > > > > > > ret = ov772x_mask_set(client, > > > EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK, > > > priv->info->edgectrl.strength); > > > if (ret < 0) > > > - goto ov772x_set_fmt_error; > > > + return ret; > > > > > > } else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) { > > > /* > > > @@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv > > > *priv, > > > EDGE_UPPER, OV772X_EDGE_UPPER_MASK, > > > priv->info->edgectrl.upper); > > > if (ret < 0) > > > - goto ov772x_set_fmt_error; > > > + return ret; > > > > > > ret = ov772x_mask_set(client, > > > EDGE_LOWER, OV772X_EDGE_LOWER_MASK, > > > priv->info->edgectrl.lower); > > > if (ret < 0) > > > - goto ov772x_set_fmt_error; > > > + return ret; > > > } > > > > > > + return 0; > > > +} > > > + > > > +static int ov772x_set_params(struct ov772x_priv *priv, > > > + const struct ov772x_color_format *cfmt, > > > + const struct ov772x_win_size *win) > > > +{ > > > + struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > > > + struct v4l2_fract tpf; > > > + int ret; > > > + u8 val; > > > + > > > + /* Reset hardware. */ > > > + ov772x_reset
Re: Are media drivers abusing of GFP_DMA? - was: Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love
Em Mon, 14 May 2018 07:35:03 -0300 Mauro Carvalho Chehab escreveu: > Hi Fabien, > > Em Mon, 14 May 2018 08:00:37 + > Fabien DESSENNE escreveu: > > > On 07/05/18 17:19, Mauro Carvalho Chehab wrote: > > > Em Mon, 07 May 2018 16:26:08 +0300 > > > Laurent Pinchart escreveu: > > > > > >> Hi Mauro, > > >> > > >> On Saturday, 5 May 2018 19:08:15 EEST Mauro Carvalho Chehab wrote: > > >>> There was a recent discussion about the use/abuse of GFP_DMA flag when > > >>> allocating memories at LSF/MM 2018 (see Luis notes enclosed). > > >>> > > >>> The idea seems to be to remove it, using CMA instead. Before doing that, > > >>> better to check if what we have on media is are valid use cases for it, > > >>> or > > >>> if it is there just due to some misunderstanding (or because it was > > >>> copied from some other code). > > >>> > > >>> Hans de Goede sent us today a patch stopping abuse at gspca, and I'm > > >>> also posting today two other patches meant to stop abuse of it on USB > > >>> drivers. Still, there are 4 platform drivers using it: > > >>> > > >>> $ git grep -l -E "GFP_DMA\\b" drivers/media/ > > >>> drivers/media/platform/omap3isp/ispstat.c > > >>> drivers/media/platform/sti/bdisp/bdisp-hw.c > > >>> drivers/media/platform/sti/hva/hva-mem.c > > > > Hi Mauro, > > > > The two STI drivers (bdisp-hw.c and hva-mem.c) are only expected to run > > on ARM platforms, not on x86. > > Since this thread deals with x86 & DMA trouble, I am not sure that we > > actually have a problem for the sti drivers. > > > > There are some other sti drivers that make use of this GFP_DMA flag > > (drivers/gpu/drm/sti/sti_*.c) and it does not seem to be a problem. > > > > Nevertheless I can see that the media sti drivers depend on COMPILE_TEST > > (which is not the case for the DRM ones). > > Would it be an acceptable solution to remove the COMPILE_TEST dependency? > > This has nothing to do with either x86 or COMPILE_TEST. The thing is > that there's a plan for removing GFP_DMA from the Kernel[1], as it was > originally meant to be used only by old PCs, where the DMA controllers > used only on the bottom 16 MB memory address (24 bits). IMHO, it is > very unlikely that any ARM SoC have such limitation. > > [1] https://lwn.net/Articles/753273/ (article will be freely available > on May, 17) Btw, you can also read about that at: https://lwn.net/Articles/753274/ > > Anyway, before the removal of GFP_DMA happens, I'd like to better > understand why we're using it at media, and if we can, instead, > set the DMA bit mask, just like almost all other media drivers > that require to confine DMA into a certain range do. In the case > of ARM, this is what we currently have: > > drivers/media/platform/exynos-gsc/gsc-core.c: > vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > drivers/media/platform/exynos4-is/fimc-core.c: > vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > drivers/media/platform/exynos4-is/fimc-is.c: > vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > drivers/media/platform/exynos4-is/fimc-lite.c: > vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > drivers/media/platform/mtk-mdp/mtk_mdp_core.c: > vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); > drivers/media/platform/omap3isp/isp.c: ret = > dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32)); > drivers/media/platform/s5p-g2d/g2d.c: > vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); > drivers/media/platform/s5p-jpeg/jpeg-core.c: > vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); > drivers/media/platform/s5p-mfc/s5p_mfc.c: > DMA_BIT_MASK(32)); > drivers/media/platform/s5p-mfc/s5p_mfc.c: > DMA_BIT_MASK(32)); > drivers/media/platform/s5p-mfc/s5p_mfc.c: > vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > > > > > BR > > > > Fabien > > > > >>> drivers/media/spi/cxd2880-spi.c > > >>> > > >>> Could you please check if GFP_DMA is really needed there, or if it is > > >>> just because of some cut-and-paste from some other place? > > >> I started looking at that for the omap3isp driver but Sakari beat me at > > >> submitting a patch. GFP_DMA isn't needed for omap3isp. > > >> > > > Thank you both for looking into it. > > > > > > Regards, > > > Mauro > > > > > > > > > > > > Thanks, > > > Mauro > > > > Thanks, > Mauro Thanks, Mauro
Re: Are media drivers abusing of GFP_DMA? - was: Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love
Hi Fabien, Em Mon, 14 May 2018 08:00:37 + Fabien DESSENNE escreveu: > On 07/05/18 17:19, Mauro Carvalho Chehab wrote: > > Em Mon, 07 May 2018 16:26:08 +0300 > > Laurent Pinchart escreveu: > > > >> Hi Mauro, > >> > >> On Saturday, 5 May 2018 19:08:15 EEST Mauro Carvalho Chehab wrote: > >>> There was a recent discussion about the use/abuse of GFP_DMA flag when > >>> allocating memories at LSF/MM 2018 (see Luis notes enclosed). > >>> > >>> The idea seems to be to remove it, using CMA instead. Before doing that, > >>> better to check if what we have on media is are valid use cases for it, or > >>> if it is there just due to some misunderstanding (or because it was > >>> copied from some other code). > >>> > >>> Hans de Goede sent us today a patch stopping abuse at gspca, and I'm > >>> also posting today two other patches meant to stop abuse of it on USB > >>> drivers. Still, there are 4 platform drivers using it: > >>> > >>> $ git grep -l -E "GFP_DMA\\b" drivers/media/ > >>> drivers/media/platform/omap3isp/ispstat.c > >>> drivers/media/platform/sti/bdisp/bdisp-hw.c > >>> drivers/media/platform/sti/hva/hva-mem.c > > Hi Mauro, > > The two STI drivers (bdisp-hw.c and hva-mem.c) are only expected to run > on ARM platforms, not on x86. > Since this thread deals with x86 & DMA trouble, I am not sure that we > actually have a problem for the sti drivers. > > There are some other sti drivers that make use of this GFP_DMA flag > (drivers/gpu/drm/sti/sti_*.c) and it does not seem to be a problem. > > Nevertheless I can see that the media sti drivers depend on COMPILE_TEST > (which is not the case for the DRM ones). > Would it be an acceptable solution to remove the COMPILE_TEST dependency? This has nothing to do with either x86 or COMPILE_TEST. The thing is that there's a plan for removing GFP_DMA from the Kernel[1], as it was originally meant to be used only by old PCs, where the DMA controllers used only on the bottom 16 MB memory address (24 bits). IMHO, it is very unlikely that any ARM SoC have such limitation. [1] https://lwn.net/Articles/753273/ (article will be freely available on May, 17) Anyway, before the removal of GFP_DMA happens, I'd like to better understand why we're using it at media, and if we can, instead, set the DMA bit mask, just like almost all other media drivers that require to confine DMA into a certain range do. In the case of ARM, this is what we currently have: drivers/media/platform/exynos-gsc/gsc-core.c: vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); drivers/media/platform/exynos4-is/fimc-core.c: vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); drivers/media/platform/exynos4-is/fimc-is.c: vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); drivers/media/platform/exynos4-is/fimc-lite.c: vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); drivers/media/platform/mtk-mdp/mtk_mdp_core.c: vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); drivers/media/platform/omap3isp/isp.c: ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32)); drivers/media/platform/s5p-g2d/g2d.c: vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); drivers/media/platform/s5p-jpeg/jpeg-core.c: vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); drivers/media/platform/s5p-mfc/s5p_mfc.c: DMA_BIT_MASK(32)); drivers/media/platform/s5p-mfc/s5p_mfc.c: DMA_BIT_MASK(32)); drivers/media/platform/s5p-mfc/s5p_mfc.c: vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > > BR > > Fabien > > >>> drivers/media/spi/cxd2880-spi.c > >>> > >>> Could you please check if GFP_DMA is really needed there, or if it is > >>> just because of some cut-and-paste from some other place? > >> I started looking at that for the omap3isp driver but Sakari beat me at > >> submitting a patch. GFP_DMA isn't needed for omap3isp. > >> > > Thank you both for looking into it. > > > > Regards, > > Mauro > > > > > > > > Thanks, > > Mauro Thanks, Mauro
Re: [PATCH v5 12/14] media: ov772x: avoid accessing registers under power saving mode
Hi Sakari, thanks for the clarification, please ignore my comment then. Thanks j On Mon, May 14, 2018 at 12:49:19PM +0300, Sakari Ailus wrote: > Hi Jacopo, > > On Mon, May 14, 2018 at 11:06:46AM +0200, jacopo mondi wrote: > > Hi Akinobu, > > > >a small nit below > > > > On Sun, May 06, 2018 at 11:19:27PM +0900, Akinobu Mita wrote: > > > The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler, > > > and the s_frame_interval() in subdev video ops could be called when the > > > device is under power saving mode. These callbacks for ov772x driver > > > cause updating H/W registers that will fail under power saving mode. > > > > > > This avoids it by not apply any changes to H/W if the device is not > > > powered > > > up. Instead the changes will be restored right after power-up. > > > > > > Cc: Jacopo Mondi > > > Cc: Laurent Pinchart > > > Cc: Hans Verkuil > > > Cc: Sakari Ailus > > > Cc: Mauro Carvalho Chehab > > > Signed-off-by: Akinobu Mita > > > --- > > > * v5 > > > - No changes > > > > > > drivers/media/i2c/ov772x.c | 79 > > > +- > > > 1 file changed, 64 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > > > index 9292a18..262a7e5 100644 > > > --- a/drivers/media/i2c/ov772x.c > > > +++ b/drivers/media/i2c/ov772x.c > > > @@ -741,19 +741,30 @@ static int ov772x_s_frame_interval(struct > > > v4l2_subdev *sd, > > > struct ov772x_priv *priv = to_ov772x(sd); > > > struct v4l2_fract *tpf = &ival->interval; > > > unsigned int fps; > > > - int ret; > > > + int ret = 0; > > > > > > fps = ov772x_select_fps(priv, tpf); > > > > > > - ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); > > > - if (ret) > > > - return ret; > > > + mutex_lock(&priv->lock); > > > + /* > > > + * If the device is not powered up by the host driver do > > > + * not apply any changes to H/W at this time. Instead > > > + * the frame rate will be restored right after power-up. > > > + */ > > > + if (priv->power_count > 0) { > > > + ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); > > > + if (ret) > > > + goto error; > > > + } > > > > > > tpf->numerator = 1; > > > tpf->denominator = fps; > > > priv->fps = fps; > > > > > > - return 0; > > > +error: > > > + mutex_unlock(&priv->lock); > > > + > > > + return ret; > > > } > > > > > > static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > > > @@ -765,6 +776,16 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > > > int ret = 0; > > > u8 val; > > > > > > + /* v4l2_ctrl_lock() locks our own mutex */ > > > + > > > + /* > > > + * If the device is not powered up by the host driver do > > > + * not apply any controls to H/W at this time. Instead > > > + * the controls will be restored right after power-up. > > > + */ > > > + if (priv->power_count == 0) > > > + return 0; > > > + > > > switch (ctrl->id) { > > > case V4L2_CID_VFLIP: > > > val = ctrl->val ? VFLIP_IMG : 0x00; > > > @@ -885,6 +906,10 @@ static int ov772x_power_off(struct ov772x_priv *priv) > > > return 0; > > > } > > > > > > +static int ov772x_set_params(struct ov772x_priv *priv, > > > + const struct ov772x_color_format *cfmt, > > > + const struct ov772x_win_size *win); > > > + > > > static int ov772x_s_power(struct v4l2_subdev *sd, int on) > > > { > > > struct ov772x_priv *priv = to_ov772x(sd); > > > @@ -895,8 +920,20 @@ static int ov772x_s_power(struct v4l2_subdev *sd, > > > int on) > > > /* If the power count is modified from 0 to != 0 or from != 0 to 0, > > >* update the power state. > > >*/ > > > - if (priv->power_count == !on) > > > - ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv); > > > + if (priv->power_count == !on) { > > > + if (on) { > > > + ret = ov772x_power_on(priv); > > > + /* > > > + * Restore the format, the frame rate, and > > > + * the controls > > > + */ > > > + if (!ret) > > > + ret = ov772x_set_params(priv, priv->cfmt, > > > + priv->win); > > > + } else { > > > + ret = ov772x_power_off(priv); > > > + } > > > + } > > > > > > if (!ret) { > > > /* Update the power count. */ > > > @@ -1163,7 +1200,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, > > > struct v4l2_mbus_framefmt *mf = &format->format; > > > const struct ov772x_color_format *cfmt; > > > const struct ov772x_win_size *win; > > > - int ret; > > > + int ret = 0; > > > > > > if (format->pad) > > > return -EINVAL; > > > @@ -1184,14 +1221,24 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, > > > return 0; > > > } > > > > > > - ret = ov772x_set_params(priv, cfmt, win); > > > - if
Re: [PATCH v5 12/14] media: ov772x: avoid accessing registers under power saving mode
Hi Jacopo, On Mon, May 14, 2018 at 11:06:46AM +0200, jacopo mondi wrote: > Hi Akinobu, > >a small nit below > > On Sun, May 06, 2018 at 11:19:27PM +0900, Akinobu Mita wrote: > > The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler, > > and the s_frame_interval() in subdev video ops could be called when the > > device is under power saving mode. These callbacks for ov772x driver > > cause updating H/W registers that will fail under power saving mode. > > > > This avoids it by not apply any changes to H/W if the device is not powered > > up. Instead the changes will be restored right after power-up. > > > > Cc: Jacopo Mondi > > Cc: Laurent Pinchart > > Cc: Hans Verkuil > > Cc: Sakari Ailus > > Cc: Mauro Carvalho Chehab > > Signed-off-by: Akinobu Mita > > --- > > * v5 > > - No changes > > > > drivers/media/i2c/ov772x.c | 79 > > +- > > 1 file changed, 64 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > > index 9292a18..262a7e5 100644 > > --- a/drivers/media/i2c/ov772x.c > > +++ b/drivers/media/i2c/ov772x.c > > @@ -741,19 +741,30 @@ static int ov772x_s_frame_interval(struct v4l2_subdev > > *sd, > > struct ov772x_priv *priv = to_ov772x(sd); > > struct v4l2_fract *tpf = &ival->interval; > > unsigned int fps; > > - int ret; > > + int ret = 0; > > > > fps = ov772x_select_fps(priv, tpf); > > > > - ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); > > - if (ret) > > - return ret; > > + mutex_lock(&priv->lock); > > + /* > > +* If the device is not powered up by the host driver do > > +* not apply any changes to H/W at this time. Instead > > +* the frame rate will be restored right after power-up. > > +*/ > > + if (priv->power_count > 0) { > > + ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); > > + if (ret) > > + goto error; > > + } > > > > tpf->numerator = 1; > > tpf->denominator = fps; > > priv->fps = fps; > > > > - return 0; > > +error: > > + mutex_unlock(&priv->lock); > > + > > + return ret; > > } > > > > static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > > @@ -765,6 +776,16 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > > int ret = 0; > > u8 val; > > > > + /* v4l2_ctrl_lock() locks our own mutex */ > > + > > + /* > > +* If the device is not powered up by the host driver do > > +* not apply any controls to H/W at this time. Instead > > +* the controls will be restored right after power-up. > > +*/ > > + if (priv->power_count == 0) > > + return 0; > > + > > switch (ctrl->id) { > > case V4L2_CID_VFLIP: > > val = ctrl->val ? VFLIP_IMG : 0x00; > > @@ -885,6 +906,10 @@ static int ov772x_power_off(struct ov772x_priv *priv) > > return 0; > > } > > > > +static int ov772x_set_params(struct ov772x_priv *priv, > > +const struct ov772x_color_format *cfmt, > > +const struct ov772x_win_size *win); > > + > > static int ov772x_s_power(struct v4l2_subdev *sd, int on) > > { > > struct ov772x_priv *priv = to_ov772x(sd); > > @@ -895,8 +920,20 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int > > on) > > /* If the power count is modified from 0 to != 0 or from != 0 to 0, > > * update the power state. > > */ > > - if (priv->power_count == !on) > > - ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv); > > + if (priv->power_count == !on) { > > + if (on) { > > + ret = ov772x_power_on(priv); > > + /* > > +* Restore the format, the frame rate, and > > +* the controls > > +*/ > > + if (!ret) > > + ret = ov772x_set_params(priv, priv->cfmt, > > + priv->win); > > + } else { > > + ret = ov772x_power_off(priv); > > + } > > + } > > > > if (!ret) { > > /* Update the power count. */ > > @@ -1163,7 +1200,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, > > struct v4l2_mbus_framefmt *mf = &format->format; > > const struct ov772x_color_format *cfmt; > > const struct ov772x_win_size *win; > > - int ret; > > + int ret = 0; > > > > if (format->pad) > > return -EINVAL; > > @@ -1184,14 +1221,24 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, > > return 0; > > } > > > > - ret = ov772x_set_params(priv, cfmt, win); > > - if (ret < 0) > > - return ret; > > - > > + mutex_lock(&priv->lock); > > + /* > > +* If the device is not powered up by the host driver do > > +* not apply any changes to H/W at this time. Instead > > +* the format will be restored right after power-up. > > +
Re: [PATCH v4 00/14] media: imx: Switch to subdev notifiers
Hi Steve, thank you for the update. On Wed, 2018-05-09 at 15:46 -0700, Steve Longerbeam wrote: > This patchset converts the imx-media driver and its dependent > subdevs to use subdev notifiers. > > There are a couple shortcomings in v4l2-core that prevented > subdev notifiers from working correctly in imx-media: > > 1. v4l2_async_notifier_fwnode_parse_endpoint() treats a fwnode >endpoint that is not connected to a remote device as an error. >But in the case of the video-mux subdev, this is not an error, >it is OK if some of the mux inputs have no connection. Also, >Documentation/devicetree/bindings/media/video-interfaces.txt explicitly >states that the 'remote-endpoint' property is optional. So the first >patch is a small modification to ignore empty endpoints in >v4l2_async_notifier_fwnode_parse_endpoint() and allow >__v4l2_async_notifier_parse_fwnode_endpoints() to continue to >parse the remaining port endpoints of the device. > > 2. In the imx-media graph, multiple subdevs will encounter the same >upstream subdev (such as the imx6-mipi-csi2 receiver), and so >v4l2_async_notifier_parse_fwnode_endpoints() will add imx6-mipi-csi2 >multiple times. This is treated as an error by >v4l2_async_notifier_register() later. > >To get around this problem, add an v4l2_async_notifier_add_subdev() >which first verifies the provided asd does not already exist in the >given notifier asd list or in other registered notifiers. If the asd >exists, the function returns -EEXIST and it's up to the caller to >decide if that is an error (in imx-media case it is never an error). > >Patches 2-5 deal with adding that support. > > 3. Patch 6 adds v4l2_async_register_fwnode_subdev(), which is a >convenience function for parsing a subdev's fwnode port endpoints >for connected remote subdevs, registering a subdev notifier, and >then registering the sub-device itself. > > The remaining patches update the subdev drivers to register a > subdev notifier with endpoint parsing, and the changes to imx-media > to support that. > > Signed-off-by: Steve Longerbeam > Acked-by: Philipp Zabel Patches 07-14 (video-mux and the imx patches) are Reviewed-by: Philipp Zabel The series is Tested-by: Philipp Zabel on i.MX6 with Toshiba TC358743 connected via MIPI CSI-2. regards Philipp
Re: [PATCH v5 08/14] media: ov772x: support device tree probing
Hi Sakari, On Mon, May 07, 2018 at 12:26:41PM +0300, Sakari Ailus wrote: > Dear Mita-san, > > On Sun, May 06, 2018 at 11:19:23PM +0900, Akinobu Mita wrote: > > The ov772x driver currently only supports legacy platform data probe. > > This change enables device tree probing. > > > > Note that the platform data probe can select auto or manual edge control > > mode, but the device tree probling can only select auto edge control mode > > for now. > > > > Cc: Jacopo Mondi > > Cc: Laurent Pinchart > > Cc: Hans Verkuil > > Cc: Sakari Ailus > > Cc: Mauro Carvalho Chehab > > Reviewed-by: Jacopo Mondi > > Signed-off-by: Akinobu Mita > > --- > > * v5 > > - Remove unnecessary space > > > > drivers/media/i2c/ov772x.c | 64 > > -- > > 1 file changed, 45 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > > index f939e28..2b02411 100644 > > --- a/drivers/media/i2c/ov772x.c > > +++ b/drivers/media/i2c/ov772x.c > > @@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > > case V4L2_CID_VFLIP: > > val = ctrl->val ? VFLIP_IMG : 0x00; > > priv->flag_vflip = ctrl->val; > > - if (priv->info->flags & OV772X_FLAG_VFLIP) > > + if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP)) > > val ^= VFLIP_IMG; > > return ov772x_mask_set(client, COM3, VFLIP_IMG, val); > > case V4L2_CID_HFLIP: > > val = ctrl->val ? HFLIP_IMG : 0x00; > > priv->flag_hflip = ctrl->val; > > - if (priv->info->flags & OV772X_FLAG_HFLIP) > > + if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP)) > > val ^= HFLIP_IMG; > > return ov772x_mask_set(client, COM3, HFLIP_IMG, val); > > case V4L2_CID_BAND_STOP_FILTER: > > @@ -914,19 +914,14 @@ static void ov772x_select_params(const struct > > v4l2_mbus_framefmt *mf, > > *win = ov772x_select_win(mf->width, mf->height); > > } > > > > -static int ov772x_set_params(struct ov772x_priv *priv, > > -const struct ov772x_color_format *cfmt, > > -const struct ov772x_win_size *win) > > +static int ov772x_edgectrl(struct ov772x_priv *priv) > > { > > struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > > - struct v4l2_fract tpf; > > int ret; > > - u8 val; > > > > - /* Reset hardware. */ > > - ov772x_reset(client); > > + if (!priv->info) > > + return 0; > > > > - /* Edge Ctrl. */ > > if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) { > > /* > > * Manual Edge Control Mode. > > @@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv *priv, > > > > ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00); > > if (ret < 0) > > - goto ov772x_set_fmt_error; > > + return ret; > > > > ret = ov772x_mask_set(client, > > EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK, > > priv->info->edgectrl.threshold); > > if (ret < 0) > > - goto ov772x_set_fmt_error; > > + return ret; > > > > ret = ov772x_mask_set(client, > > EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK, > > priv->info->edgectrl.strength); > > if (ret < 0) > > - goto ov772x_set_fmt_error; > > + return ret; > > > > } else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) { > > /* > > @@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv *priv, > > EDGE_UPPER, OV772X_EDGE_UPPER_MASK, > > priv->info->edgectrl.upper); > > if (ret < 0) > > - goto ov772x_set_fmt_error; > > + return ret; > > > > ret = ov772x_mask_set(client, > > EDGE_LOWER, OV772X_EDGE_LOWER_MASK, > > priv->info->edgectrl.lower); > > if (ret < 0) > > - goto ov772x_set_fmt_error; > > + return ret; > > } > > > > + return 0; > > +} > > + > > +static int ov772x_set_params(struct ov772x_priv *priv, > > +const struct ov772x_color_format *cfmt, > > +const struct ov772x_win_size *win) > > +{ > > + struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > > + struct v4l2_fract tpf; > > + int ret; > > + u8 val; > > + > > + /* Reset hardware. */ > > + ov772x_reset(client); > > + > > + /* Edge Ctrl. */ > > + ret = ov772x_edgectrl(priv); > > + if (ret < 0) > > + return ret; > > + > > /* Format and window size. */ > > ret = o
Re: [PATCH v5 14/14] media: ov772x: create subdevice device node
Hi Akinobu, thanks for the patch. On Sun, May 06, 2018 at 11:19:29PM +0900, Akinobu Mita wrote: > Set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so that the > subdevice device node is created. > > Cc: Jacopo Mondi > Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita Reviewed-by: Jacopo Mondi Thanks j > --- > * v5 > - No changes > > drivers/media/i2c/ov772x.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 4b479f9..f7f4fe6 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -1409,6 +1409,7 @@ static int ov772x_probe(struct i2c_client *client, > mutex_init(&priv->lock); > > v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subdev_ops); > + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > v4l2_ctrl_handler_init(&priv->hdl, 3); > /* Use our mutex for the controls */ > priv->hdl.lock = &priv->lock; > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [PATCH v5 13/14] media: ov772x: make set_fmt() and s_frame_interval() return -EBUSY while streaming
Hi Akinobu, thanks for the patch On Sun, May 06, 2018 at 11:19:28PM +0900, Akinobu Mita wrote: > The ov772x driver is going to offer a V4L2 sub-device interface, so > changing the output data format and the frame interval on this sub-device > can be made anytime. However, these requests are preferred to fail while > the video stream on the device is active. > > Cc: Jacopo Mondi > Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita Reviewed-by: Jacopo Mondi Thanks j > --- > * v5: > - Make s_frame_interval() return -EBUSY while streaming > > drivers/media/i2c/ov772x.c | 43 +-- > 1 file changed, 33 insertions(+), 10 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 262a7e5..4b479f9 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -424,9 +424,10 @@ struct ov772x_priv { > /* band_filter = COM8[5] ? 256 - BDBASE : 0 */ > struct v4l2_ctrl *band_filter_ctrl; > unsigned int fps; > - /* lock to protect power_count */ > + /* lock to protect power_count and streaming */ > struct mutex lock; > int power_count; > + int streaming; > #ifdef CONFIG_MEDIA_CONTROLLER > struct media_pad pad; > #endif > @@ -603,18 +604,28 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int > enable) > { > struct i2c_client *client = v4l2_get_subdevdata(sd); > struct ov772x_priv *priv = to_ov772x(sd); > + int ret = 0; > > - if (!enable) { > - ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE); > - return 0; > - } > + mutex_lock(&priv->lock); > > - ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, 0); > + if (priv->streaming == enable) > + goto done; > > - dev_dbg(&client->dev, "format %d, win %s\n", > - priv->cfmt->code, priv->win->name); > + ret = ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, > + enable ? 0 : SOFT_SLEEP_MODE); > + if (ret) > + goto done; > > - return 0; > + if (enable) { > + dev_dbg(&client->dev, "format %d, win %s\n", > + priv->cfmt->code, priv->win->name); > + } > + priv->streaming = enable; > + > +done: > + mutex_unlock(&priv->lock); > + > + return ret; > } > > static unsigned int ov772x_select_fps(struct ov772x_priv *priv, > @@ -743,9 +754,15 @@ static int ov772x_s_frame_interval(struct v4l2_subdev > *sd, > unsigned int fps; > int ret = 0; > > + mutex_lock(&priv->lock); > + > + if (priv->streaming) { > + ret = -EBUSY; > + goto error; > + } > + > fps = ov772x_select_fps(priv, tpf); > > - mutex_lock(&priv->lock); > /* >* If the device is not powered up by the host driver do >* not apply any changes to H/W at this time. Instead > @@ -1222,6 +1239,12 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, > } > > mutex_lock(&priv->lock); > + > + if (priv->streaming) { > + ret = -EBUSY; > + goto error; > + } > + > /* >* If the device is not powered up by the host driver do >* not apply any changes to H/W at this time. Instead > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [PATCH v5 12/14] media: ov772x: avoid accessing registers under power saving mode
Hi Akinobu, a small nit below On Sun, May 06, 2018 at 11:19:27PM +0900, Akinobu Mita wrote: > The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler, > and the s_frame_interval() in subdev video ops could be called when the > device is under power saving mode. These callbacks for ov772x driver > cause updating H/W registers that will fail under power saving mode. > > This avoids it by not apply any changes to H/W if the device is not powered > up. Instead the changes will be restored right after power-up. > > Cc: Jacopo Mondi > Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita > --- > * v5 > - No changes > > drivers/media/i2c/ov772x.c | 79 > +- > 1 file changed, 64 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 9292a18..262a7e5 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -741,19 +741,30 @@ static int ov772x_s_frame_interval(struct v4l2_subdev > *sd, > struct ov772x_priv *priv = to_ov772x(sd); > struct v4l2_fract *tpf = &ival->interval; > unsigned int fps; > - int ret; > + int ret = 0; > > fps = ov772x_select_fps(priv, tpf); > > - ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); > - if (ret) > - return ret; > + mutex_lock(&priv->lock); > + /* > + * If the device is not powered up by the host driver do > + * not apply any changes to H/W at this time. Instead > + * the frame rate will be restored right after power-up. > + */ > + if (priv->power_count > 0) { > + ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); > + if (ret) > + goto error; > + } > > tpf->numerator = 1; > tpf->denominator = fps; > priv->fps = fps; > > - return 0; > +error: > + mutex_unlock(&priv->lock); > + > + return ret; > } > > static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > @@ -765,6 +776,16 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > int ret = 0; > u8 val; > > + /* v4l2_ctrl_lock() locks our own mutex */ > + > + /* > + * If the device is not powered up by the host driver do > + * not apply any controls to H/W at this time. Instead > + * the controls will be restored right after power-up. > + */ > + if (priv->power_count == 0) > + return 0; > + > switch (ctrl->id) { > case V4L2_CID_VFLIP: > val = ctrl->val ? VFLIP_IMG : 0x00; > @@ -885,6 +906,10 @@ static int ov772x_power_off(struct ov772x_priv *priv) > return 0; > } > > +static int ov772x_set_params(struct ov772x_priv *priv, > + const struct ov772x_color_format *cfmt, > + const struct ov772x_win_size *win); > + > static int ov772x_s_power(struct v4l2_subdev *sd, int on) > { > struct ov772x_priv *priv = to_ov772x(sd); > @@ -895,8 +920,20 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int on) > /* If the power count is modified from 0 to != 0 or from != 0 to 0, >* update the power state. >*/ > - if (priv->power_count == !on) > - ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv); > + if (priv->power_count == !on) { > + if (on) { > + ret = ov772x_power_on(priv); > + /* > + * Restore the format, the frame rate, and > + * the controls > + */ > + if (!ret) > + ret = ov772x_set_params(priv, priv->cfmt, > + priv->win); > + } else { > + ret = ov772x_power_off(priv); > + } > + } > > if (!ret) { > /* Update the power count. */ > @@ -1163,7 +1200,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, > struct v4l2_mbus_framefmt *mf = &format->format; > const struct ov772x_color_format *cfmt; > const struct ov772x_win_size *win; > - int ret; > + int ret = 0; > > if (format->pad) > return -EINVAL; > @@ -1184,14 +1221,24 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd, > return 0; > } > > - ret = ov772x_set_params(priv, cfmt, win); > - if (ret < 0) > - return ret; > - > + mutex_lock(&priv->lock); > + /* > + * If the device is not powered up by the host driver do > + * not apply any changes to H/W at this time. Instead > + * the format will be restored right after power-up. > + */ > + if (priv->power_count > 0) { > + ret = ov772x_set_params(priv, cfmt, win); > + if (ret < 0) > + goto error; > + } > priv->win = wi
Re: [PATCH v5 10/14] media: ov772x: reconstruct s_frame_interval()
Hi Akinobu, thanks for resending On Sun, May 06, 2018 at 11:19:25PM +0900, Akinobu Mita wrote: > This splits the s_frame_interval() in subdev video ops into selecting the > frame interval and setting up the registers. > > This is a preparatory change to avoid accessing registers under power > saving mode. > > Cc: Jacopo Mondi > Cc: Laurent Pinchart > Cc: Hans Verkuil > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita Reviewed-by: Jacopo Mondi Thanks j > --- > * v5 > - Align arguments to open parenthesis > - Sort variable declarations > > drivers/media/i2c/ov772x.c | 56 > +- > 1 file changed, 35 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 6c0c792..92ad13f 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -617,25 +617,16 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int > enable) > return 0; > } > > -static int ov772x_set_frame_rate(struct ov772x_priv *priv, > - struct v4l2_fract *tpf, > - const struct ov772x_color_format *cfmt, > - const struct ov772x_win_size *win) > +static unsigned int ov772x_select_fps(struct ov772x_priv *priv, > + struct v4l2_fract *tpf) > { > - struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > - unsigned long fin = clk_get_rate(priv->clk); > unsigned int fps = tpf->numerator ? > tpf->denominator / tpf->numerator : > tpf->denominator; > unsigned int best_diff; > - unsigned int fsize; > - unsigned int pclk; > unsigned int diff; > unsigned int idx; > unsigned int i; > - u8 clkrc = 0; > - u8 com4 = 0; > - int ret; > > /* Approximate to the closest supported frame interval. */ > best_diff = ~0L; > @@ -646,7 +637,25 @@ static int ov772x_set_frame_rate(struct ov772x_priv > *priv, > best_diff = diff; > } > } > - fps = ov772x_frame_intervals[idx]; > + > + return ov772x_frame_intervals[idx]; > +} > + > +static int ov772x_set_frame_rate(struct ov772x_priv *priv, > + unsigned int fps, > + const struct ov772x_color_format *cfmt, > + const struct ov772x_win_size *win) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > + unsigned long fin = clk_get_rate(priv->clk); > + unsigned int best_diff; > + unsigned int fsize; > + unsigned int pclk; > + unsigned int diff; > + unsigned int i; > + u8 clkrc = 0; > + u8 com4 = 0; > + int ret; > > /* Use image size (with blankings) to calculate desired pixel clock. */ > switch (cfmt->com7 & OFMT_MASK) { > @@ -711,10 +720,6 @@ static int ov772x_set_frame_rate(struct ov772x_priv > *priv, > if (ret < 0) > return ret; > > - tpf->numerator = 1; > - tpf->denominator = fps; > - priv->fps = tpf->denominator; > - > return 0; > } > > @@ -735,8 +740,20 @@ static int ov772x_s_frame_interval(struct v4l2_subdev > *sd, > { > struct ov772x_priv *priv = to_ov772x(sd); > struct v4l2_fract *tpf = &ival->interval; > + unsigned int fps; > + int ret; > + > + fps = ov772x_select_fps(priv, tpf); > + > + ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); > + if (ret) > + return ret; > > - return ov772x_set_frame_rate(priv, tpf, priv->cfmt, priv->win); > + tpf->numerator = 1; > + tpf->denominator = fps; > + priv->fps = fps; > + > + return 0; > } > > static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > @@ -993,7 +1010,6 @@ static int ov772x_set_params(struct ov772x_priv *priv, >const struct ov772x_win_size *win) > { > struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > - struct v4l2_fract tpf; > int ret; > u8 val; > > @@ -1075,9 +1091,7 @@ static int ov772x_set_params(struct ov772x_priv *priv, > goto ov772x_set_fmt_error; > > /* COM4, CLKRC: Set pixel clock and framerate. */ > - tpf.numerator = 1; > - tpf.denominator = priv->fps; > - ret = ov772x_set_frame_rate(priv, &tpf, cfmt, win); > + ret = ov772x_set_frame_rate(priv, priv->fps, cfmt, win); > if (ret < 0) > goto ov772x_set_fmt_error; > > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [PATCH 2/5] media: rcar-vin: Add digital input subdevice parsing
Hi Niklas, On Fri, May 11, 2018 at 01:01:24PM +0200, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your work! > > The comments here apply to both 2/5 and 3/5. > > On 2018-05-11 11:59:38 +0200, Jacopo Mondi wrote: > > Add support for digital input subdevices to Gen-3 rcar-vin. > > The Gen-3, media-controller compliant, version has so far only accepted > > CSI-2 input subdevices. Remove assumptions on the supported bus_type and > > accepted number of subdevices, and allow digital input connections on > > port@0. > > I feel this much more complex then it has to be. You defer parsing the > DT graph until all VIN's are probed just like it's done for the port@1 > parsing. For the CSI-2 endpoints in port@1 this is needed as they are > shared for all VIN's in the group. While for the parallel input this is > local for each VIN and could be parsed at probe time for each individual > VIN. As a bonus for doing that I think you could reuse the parallel > parsing functions from the Gen2 code whit small additions. > > Maybe something like this can be done: > > - At each VIN probe() rework the block > > if (vin->info->use_mc) > ret = rvin_mc_init(vin); > else > ret = rvin_digital_graph_init(vin); > > To: > ret = rvin_digital_graph_init(vin); > ... > ret = rvin_mc_init(vin); > ... > > - Then in rvin_digital_graph_init() don't call > v4l2_async_notifier_register() if vin->info->use_mc is set. > > And instead as a last step in rvin_mc_init() call > v4l2_async_notifier_register() for each vin->notifier that contains a > subdevice. > > - As a last step add a check for vin->info->use_mc in > rvin_digital_notify_complete() and handle the case for Gen2 behavior > (what it does now) and Gen3 behavior (adding the MC link). > > > I think my doing this you could greatly simplify the process of handling > the parallel subdevice. > > Please see bellow for one additional comment. Thanks for the suggestion. My understanding was that the 'Gen2' was planned for removal in the long term, and we should have paved the road for a media-controller-only version of the driver. But I admit I didn't even try to re-use that code part, I'll give your suggestions a spin, if this won't prevent future removal of the non-mc part of the driver. > > > > > Signed-off-by: Jacopo Mondi > > --- > > drivers/media/platform/rcar-vin/rcar-core.c | 166 > > +++- > > drivers/media/platform/rcar-vin/rcar-vin.h | 13 +++ > > 2 files changed, 153 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c > > b/drivers/media/platform/rcar-vin/rcar-core.c > > index e547ef7..105b6b6 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-core.c > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c > > @@ -697,29 +697,21 @@ static const struct v4l2_async_notifier_operations > > rvin_group_notify_ops = { > > .complete = rvin_group_notify_complete, [snip] > > + > > + switch (vep->base.port) { > > + case RVIN_PORT_DIGITAL: > > + ret = rvin_mc_parse_of_digital(vin, vep, asd); > > + break; > > + case RVIN_PORT_CSI2: > > + default: > > + ret = rvin_mc_parse_of_csi2(vin, vep, asd); > > + break; > > + } > > + /* [snip] > > + > > + switch (ep.port) { > > + case RVIN_PORT_DIGITAL: > > + asd_struct_size = sizeof(struct rvin_graph_entity); > > + break; > > + case RVIN_PORT_CSI2: > > + asd_struct_size = sizeof(struct v4l2_async_subdev); > > + break; > > + default: > > }; [snip] > > > > /** > > + * enum rvin_port_id > > + * > > + * List the available VIN port functions. > > + * > > + * RVIN_PORT_DIGITAL - Input port for digital video connection > > + * RVIN_PORT_CSI2 - Input port for CSI-2 video connection > > + */ > > +enum rvin_port_id { > > + RVIN_PORT_DIGITAL, > > + RVIN_PORT_CSI2 > > +}; > > This enum is never used in the patch-set is it used later by a different > patch-set or a left over from refactoring? I see two occurencies each of this enumeration members in this patch. I left them un-cut above. Thanks j signature.asc Description: PGP signature
Re: [PATCH v15 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver
On Sun, May 13, 2018 at 09:19:17PM +0200, Niklas Söderlund wrote: > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver > supports the R-Car Gen3 SoCs where separate CSI-2 hardware blocks are > connected between the video sources and the video grabbers (VIN). > > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP. > > Signed-off-by: Niklas Söderlund Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: Are media drivers abusing of GFP_DMA? - was: Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love
On 07/05/18 17:19, Mauro Carvalho Chehab wrote: > Em Mon, 07 May 2018 16:26:08 +0300 > Laurent Pinchart escreveu: > >> Hi Mauro, >> >> On Saturday, 5 May 2018 19:08:15 EEST Mauro Carvalho Chehab wrote: >>> There was a recent discussion about the use/abuse of GFP_DMA flag when >>> allocating memories at LSF/MM 2018 (see Luis notes enclosed). >>> >>> The idea seems to be to remove it, using CMA instead. Before doing that, >>> better to check if what we have on media is are valid use cases for it, or >>> if it is there just due to some misunderstanding (or because it was >>> copied from some other code). >>> >>> Hans de Goede sent us today a patch stopping abuse at gspca, and I'm >>> also posting today two other patches meant to stop abuse of it on USB >>> drivers. Still, there are 4 platform drivers using it: >>> >>> $ git grep -l -E "GFP_DMA\\b" drivers/media/ >>> drivers/media/platform/omap3isp/ispstat.c >>> drivers/media/platform/sti/bdisp/bdisp-hw.c >>> drivers/media/platform/sti/hva/hva-mem.c Hi Mauro, The two STI drivers (bdisp-hw.c and hva-mem.c) are only expected to run on ARM platforms, not on x86. Since this thread deals with x86 & DMA trouble, I am not sure that we actually have a problem for the sti drivers. There are some other sti drivers that make use of this GFP_DMA flag (drivers/gpu/drm/sti/sti_*.c) and it does not seem to be a problem. Nevertheless I can see that the media sti drivers depend on COMPILE_TEST (which is not the case for the DRM ones). Would it be an acceptable solution to remove the COMPILE_TEST dependency? BR Fabien >>> drivers/media/spi/cxd2880-spi.c >>> >>> Could you please check if GFP_DMA is really needed there, or if it is >>> just because of some cut-and-paste from some other place? >> I started looking at that for the omap3isp driver but Sakari beat me at >> submitting a patch. GFP_DMA isn't needed for omap3isp. >> > Thank you both for looking into it. > > Regards, > Mauro > > > > Thanks, > Mauro