From patchwork Thu Oct 3 19:08:32 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ezequiel Garcia X-Patchwork-Id: 11173153 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BE52F14DB for ; Thu, 3 Oct 2019 19:09:16 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9A4C521A4C for ; Thu, 3 Oct 2019 19:09:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="FjsuE1Hm" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9A4C521A4C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=vqJF7DCpWJ3giblPrtqN+Y2IuS4df42KTulHa5NHP5Q=; b=FjsuE1HmY/ltMj olJzPUqdm2g/D3n1fJogh1OKeS9hljidIDcBizt16ScDFp5S2Op9r/KVh43YJl/9yk4BcmfMq1xP9 XLZQ/MUAaeanjTmibWKxaXsN1g8+YYzkEplNOkIx16wopQSH9SbktVcyoIPEiR5wHUnJikJF1eVNy 1C9BIXXubhTuH2mwqajgMa9R+/3Wu0rKs0lnHyXYEqnOrxZJ4g63SmRoh/mNQGpeNykU0138UvvNT L3nfosa9MDNaBP3CBFPsdz/qYTXDCtsT4e6CGccCSJzxZGAaGAQ+B9IuFHw659X/oidC563kg+lGu DUJK0j/+Rihrcv9naD0Q==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.2 #3 (Red Hat Linux)) id 1iG6TR-0004SK-UF; Thu, 03 Oct 2019 19:09:13 +0000 Received: from bhuna.collabora.co.uk ([46.235.227.227]) by bombadil.infradead.org with esmtps (Exim 4.92.2 #3 (Red Hat Linux)) id 1iG6TN-0004Nl-8m for linux-rockchip@lists.infradead.org; Thu, 03 Oct 2019 19:09:11 +0000 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: ezequiel) with ESMTPSA id EA93028FEA5 From: Ezequiel Garcia To: linux-media@vger.kernel.org Subject: [PATCH v2 3/4] media: hantro: Rework media topology Date: Thu, 3 Oct 2019 16:08:32 -0300 Message-Id: <20191003190833.29046-4-ezequiel@collabora.com> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20191003190833.29046-1-ezequiel@collabora.com> References: <20191003190833.29046-1-ezequiel@collabora.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191003_120909_593410_46AD1763 X-CRM114-Status: GOOD ( 20.54 ) X-Spam-Score: -0.0 (/) X-Spam-Report: SpamAssassin version 3.4.2 on bombadil.infradead.org summary: Content analysis details: (-0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [46.235.227.227 listed in list.dnswl.org] -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record 0.0 UNPARSEABLE_RELAY Informational: message has unparseable relay lines X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Heiko Stuebner , Jonas Karlman , linux-kernel@vger.kernel.org, Tomasz Figa , linux-rockchip@lists.infradead.org, Helen Koike , Boris Brezillon , Philipp Zabel , kernel@collabora.com, Ezequiel Garcia , Chris Healy Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org As suggested by Helen Koike, the decoder processing entity can be modeled as a V4L2 subdevice. This change will allow to describe more complex topology and/or behavior to userspace, starting with the post-processing feature, which will be soon introduced. For now, introduce a simple subdevice, maintaining an immutable topology, and now exposing the subdevices to userspace. Suggested-by: Helen Koike Signed-off-by: Ezequiel Garcia --- drivers/staging/media/hantro/hantro.h | 21 +- drivers/staging/media/hantro/hantro_drv.c | 250 +++++++++++++++------ drivers/staging/media/hantro/hantro_v4l2.c | 18 +- 3 files changed, 205 insertions(+), 84 deletions(-) diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h index deb90ae37859..15506b9a34f4 100644 --- a/drivers/staging/media/hantro/hantro.h +++ b/drivers/staging/media/hantro/hantro.h @@ -124,25 +124,19 @@ struct hantro_ctrl { * %MEDIA_ENT_F_PROC_VIDEO_DECODER) * @vdev: &struct video_device that exposes the encoder or * decoder functionality - * @source_pad: &struct media_pad with the source pad. - * @sink: &struct media_entity pointer with the sink entity - * @sink_pad: &struct media_pad with the sink pad. - * @proc: &struct media_entity pointer with the M2M device itself. - * @proc_pads: &struct media_pad with the @proc pads. - * @intf_devnode: &struct media_intf devnode pointer with the interface - * with controls the M2M device. + * @vdev_pads: &struct media_pad with the @vdev pads. + * @sd_proc: &struct v4l2_subdev exposing the encoder or decoder sub-device + * @sd_proc_pads: &struct media_pad with the @sd_proc pads. * * Contains everything needed to attach the video device to the media device. */ struct hantro_func { unsigned int id; struct video_device vdev; - struct media_pad source_pad; - struct media_entity sink; - struct media_pad sink_pad; - struct media_entity proc; - struct media_pad proc_pads[2]; - struct media_intf_devnode *intf_devnode; + struct media_pad vdev_pads[2]; + + struct v4l2_subdev sd_proc; + struct media_pad sd_proc_pads[2]; }; static inline struct hantro_func * @@ -220,6 +214,7 @@ struct hantro_dev { struct hantro_ctx { struct hantro_dev *dev; struct v4l2_fh fh; + struct media_pipeline pipe; u32 sequence_cap; u32 sequence_out; diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c index 26108c96b674..35beb5a9bf52 100644 --- a/drivers/staging/media/hantro/hantro_drv.c +++ b/drivers/staging/media/hantro/hantro_drv.c @@ -488,9 +488,67 @@ static const struct of_device_id of_hantro_match[] = { }; MODULE_DEVICE_TABLE(of, of_hantro_match); +static int link_setup(struct media_entity *entity, + const struct media_pad *local, + const struct media_pad *remote, u32 flags) +{ + /* empty for now */ + return 0; +} + +static const struct media_entity_operations sd_mops = { + .link_setup = link_setup, +}; + +static const struct v4l2_subdev_ops sd_ops = { + /* empty */ +}; + +static int +hantro_subdev_register(struct hantro_dev *vpu, + struct hantro_func *func, + struct v4l2_subdev *sd, + const char *const name, + u32 function, + struct media_pad *pads, + const struct v4l2_subdev_internal_ops *sd_int_ops, + const struct v4l2_subdev_ops *sd_ops) +{ + int ret; + + /* Initialize the subdev */ + v4l2_subdev_init(sd, sd_ops); + sd->internal_ops = sd_int_ops; + sd->entity.function = function; + sd->entity.ops = &sd_mops; + sd->owner = THIS_MODULE; + strscpy(sd->name, name, sizeof(sd->name)); + v4l2_set_subdevdata(sd, vpu); + + if (sd->ctrl_handler) + sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS; + + /* Initialize the media entity */ + pads[0].flags = MEDIA_PAD_FL_SINK; + pads[1].flags = MEDIA_PAD_FL_SOURCE; + ret = media_entity_pads_init(&sd->entity, 2, pads); + if (ret) + return ret; + + /* Register the subdev with the v4l2 and the media frameworks */ + ret = v4l2_device_register_subdev(&vpu->v4l2_dev, sd); + if (ret) { + v4l2_err(&vpu->v4l2_dev, + "%s: subdev register failed (err=%d)\n", + name, ret); + return ret; + } + + return 0; +} + static int hantro_register_entity(struct media_device *mdev, struct media_entity *entity, - const char *entity_name, struct media_pad *pads, int num_pads, int function, struct video_device *vdev) { @@ -503,8 +561,7 @@ static int hantro_register_entity(struct media_device *mdev, entity->info.dev.minor = vdev->minor; } - name = devm_kasprintf(mdev->dev, GFP_KERNEL, "%s-%s", vdev->name, - entity_name); + name = devm_kasprintf(mdev->dev, GFP_KERNEL, "%s", vdev->name); if (!name) return -ENOMEM; @@ -522,61 +579,132 @@ static int hantro_register_entity(struct media_device *mdev, return 0; } +#define HANTRO_LINK_(_src, _sink, link_flags) { \ + .source = _src, \ + .sink = _sink, \ + .flags = link_flags, \ +} + +#define HANTRO_LINK_IM(_src, _sink) \ + HANTRO_LINK_(_src, _sink, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE) + +#define HANTRO_LINK_EN(_src, _sink) \ + HANTRO_LINK_(_src, _sink, MEDIA_LNK_FL_ENABLED) + +#define HANTRO_LINK(_src, _sink) \ + HANTRO_LINK_(_src, _sink, 0) + +#define HANTRO_SUBDEV(_subdev, _name, _function, _pads) { \ + .subdev = _subdev, \ + .name = _name, \ + .function = _function, \ + .pads = _pads, \ +} + static int hantro_attach_func(struct hantro_dev *vpu, struct hantro_func *func) { struct media_device *mdev = &vpu->mdev; struct media_link *link; + unsigned int i, num_subdevs, num_links; int ret; - /* Create the three encoder entities with their pads */ - func->source_pad.flags = MEDIA_PAD_FL_SOURCE; - ret = hantro_register_entity(mdev, &func->vdev.entity, "source", - &func->source_pad, 1, MEDIA_ENT_F_IO_V4L, - &func->vdev); - if (ret) - return ret; + /* + * In order to ease the media topology setup, + * define a couple compound types. Keep these + * types local, as they are not needed them + * elsewhere. + */ + struct hantro_subdev { + struct v4l2_subdev *subdev; + struct media_pad *pads; + const char *name; + u32 function; + }; + struct hantro_link { + struct media_entity *source; + struct media_entity *sink; + u32 flags; + }; + const struct hantro_subdev decoder_subdevs[] = { + HANTRO_SUBDEV(&func->sd_proc, "decoder", func->id, + func->sd_proc_pads), + }; + const struct hantro_subdev encoder_subdevs[] = { + HANTRO_SUBDEV(&func->sd_proc, "encoder", func->id, + func->sd_proc_pads), + }; + const struct hantro_link decoder_links[] = { + /* decoder -> vdev */ + HANTRO_LINK_IM(&func->sd_proc.entity, &func->vdev.entity), + /* vdev -> decoder */ + HANTRO_LINK_IM(&func->vdev.entity, &func->sd_proc.entity), + }; + const struct hantro_link encoder_links[] = { + /* encoder -> vdev */ + HANTRO_LINK_IM(&func->sd_proc.entity, &func->vdev.entity), + /* vdev -> encoder */ + HANTRO_LINK_IM(&func->vdev.entity, &func->sd_proc.entity), + }; + const struct hantro_subdev *subdevs; + const struct hantro_link *links; - func->proc_pads[0].flags = MEDIA_PAD_FL_SINK; - func->proc_pads[1].flags = MEDIA_PAD_FL_SOURCE; - ret = hantro_register_entity(mdev, &func->proc, "proc", - func->proc_pads, 2, func->id, - &func->vdev); - if (ret) - goto err_rel_entity0; + if (func->id == MEDIA_ENT_F_PROC_VIDEO_ENCODER) { + subdevs = encoder_subdevs; + links = encoder_links; + num_subdevs = ARRAY_SIZE(encoder_subdevs); + num_links = ARRAY_SIZE(encoder_links); + } else { + subdevs = decoder_subdevs; + links = decoder_links; + num_subdevs = ARRAY_SIZE(decoder_subdevs); + num_links = ARRAY_SIZE(decoder_links); + } - func->sink_pad.flags = MEDIA_PAD_FL_SINK; - ret = hantro_register_entity(mdev, &func->sink, "sink", - &func->sink_pad, 1, MEDIA_ENT_F_IO_V4L, + for (i = 0; i < num_subdevs; i++) { + const struct hantro_subdev *subdev = &subdevs[i]; + + ret = hantro_subdev_register(vpu, func, subdev->subdev, + subdev->name, subdev->function, + subdev->pads, + NULL, &sd_ops); + if (ret) + goto err_unreg_subdevs; + } + + func->vdev_pads[0].flags = MEDIA_PAD_FL_SINK; + func->vdev_pads[1].flags = MEDIA_PAD_FL_SOURCE; + ret = hantro_register_entity(mdev, &func->vdev.entity, + func->vdev_pads, 2, + MEDIA_ENT_F_IO_V4L, &func->vdev); if (ret) - goto err_rel_entity1; + goto err_unreg_subdevs; - /* Connect the three entities */ - ret = media_create_pad_link(&func->vdev.entity, 0, &func->proc, 1, - MEDIA_LNK_FL_IMMUTABLE | - MEDIA_LNK_FL_ENABLED); - if (ret) - goto err_rel_entity2; + for (i = 0; i < num_links; i++) { + const struct hantro_link *link = &links[i]; - ret = media_create_pad_link(&func->proc, 0, &func->sink, 0, - MEDIA_LNK_FL_IMMUTABLE | - MEDIA_LNK_FL_ENABLED); - if (ret) - goto err_rm_links0; + ret = media_create_pad_link(link->source, 1, + link->sink, 0, + link->flags); + if (ret) { + ret = -ENOMEM; + goto err_unreg_entity; + } + } - /* Create video interface */ - func->intf_devnode = media_devnode_create(mdev, MEDIA_INTF_T_V4L_VIDEO, - 0, VIDEO_MAJOR, - func->vdev.minor); - if (!func->intf_devnode) { + /* Create the video device interface and link it. */ + func->vdev.intf_devnode = + media_devnode_create(mdev, MEDIA_INTF_T_V4L_VIDEO, + 0, VIDEO_MAJOR, + func->vdev.minor); + if (!func->vdev.intf_devnode) { ret = -ENOMEM; - goto err_rm_links1; + goto err_unreg_entity; } - /* Connect the two DMA engines to the interface */ link = media_create_intf_link(&func->vdev.entity, - &func->intf_devnode->intf, + &func->vdev.intf_devnode->intf, MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED); if (!link) { @@ -584,45 +712,22 @@ static int hantro_attach_func(struct hantro_dev *vpu, goto err_rm_devnode; } - link = media_create_intf_link(&func->sink, &func->intf_devnode->intf, - MEDIA_LNK_FL_IMMUTABLE | - MEDIA_LNK_FL_ENABLED); - if (!link) { - ret = -ENOMEM; - goto err_rm_devnode; - } return 0; - err_rm_devnode: - media_devnode_remove(func->intf_devnode); - -err_rm_links1: - media_entity_remove_links(&func->sink); - -err_rm_links0: - media_entity_remove_links(&func->proc); - media_entity_remove_links(&func->vdev.entity); - -err_rel_entity2: - media_device_unregister_entity(&func->sink); - -err_rel_entity1: - media_device_unregister_entity(&func->proc); - -err_rel_entity0: + media_devnode_remove(func->vdev.intf_devnode); +err_unreg_entity: media_device_unregister_entity(&func->vdev.entity); +err_unreg_subdevs: + for (i = 0; i < num_subdevs; i++) + v4l2_device_unregister_subdev(subdevs[i].subdev); return ret; } static void hantro_detach_func(struct hantro_func *func) { - media_devnode_remove(func->intf_devnode); - media_entity_remove_links(&func->sink); - media_entity_remove_links(&func->proc); - media_entity_remove_links(&func->vdev.entity); - media_device_unregister_entity(&func->sink); - media_device_unregister_entity(&func->proc); + media_devnode_remove(func->vdev.intf_devnode); media_device_unregister_entity(&func->vdev.entity); + v4l2_device_unregister_subdev(&func->sd_proc); } static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid) @@ -866,6 +971,11 @@ static int hantro_probe(struct platform_device *pdev) goto err_rm_dec_func; } + ret = v4l2_device_register_subdev_nodes(&vpu->v4l2_dev); + if (ret) { + v4l2_err(&vpu->v4l2_dev, "Failed to register subdev nodes\n"); + goto err_rm_dec_func; + } return 0; err_rm_dec_func: diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c index 238e53b28f8f..58fa4b52275b 100644 --- a/drivers/staging/media/hantro/hantro_v4l2.c +++ b/drivers/staging/media/hantro/hantro_v4l2.c @@ -594,6 +594,7 @@ static bool hantro_vq_is_coded(struct vb2_queue *q) static int hantro_start_streaming(struct vb2_queue *q, unsigned int count) { struct hantro_ctx *ctx = vb2_get_drv_priv(q); + struct media_entity *entity = &ctx->fh.vdev->entity; int ret = 0; if (V4L2_TYPE_IS_OUTPUT(q->type)) @@ -601,6 +602,11 @@ static int hantro_start_streaming(struct vb2_queue *q, unsigned int count) else ctx->sequence_cap = 0; + /* Start the media pipeline */ + ret = media_pipeline_start(entity, &ctx->pipe); + if (ret) + return ret; + if (hantro_vq_is_coded(q)) { enum hantro_codec_mode codec_mode; @@ -611,11 +617,18 @@ static int hantro_start_streaming(struct vb2_queue *q, unsigned int count) vpu_debug(4, "Codec mode = %d\n", codec_mode); ctx->codec_ops = &ctx->dev->variant->codec_ops[codec_mode]; - if (ctx->codec_ops->init) + if (ctx->codec_ops->init) { ret = ctx->codec_ops->init(ctx); + if (ret) + goto err_pipe_stop; + } } return ret; + +err_pipe_stop: + media_pipeline_stop(entity); + return ret; } static void @@ -639,12 +652,15 @@ hantro_return_bufs(struct vb2_queue *q, static void hantro_stop_streaming(struct vb2_queue *q) { struct hantro_ctx *ctx = vb2_get_drv_priv(q); + struct media_entity *entity = &ctx->fh.vdev->entity; if (hantro_vq_is_coded(q)) { if (ctx->codec_ops && ctx->codec_ops->exit) ctx->codec_ops->exit(ctx); } + media_pipeline_stop(entity); + /* * The mem2mem framework calls v4l2_m2m_cancel_job before * .stop_streaming, so there isn't any job running and