From patchwork Fri Mar 21 08:37:15 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arun Kumar K X-Patchwork-Id: 3871651 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id BAA65BF540 for ; Fri, 21 Mar 2014 08:37:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B100A20279 for ; Fri, 21 Mar 2014 08:37:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9B52520274 for ; Fri, 21 Mar 2014 08:37:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760170AbaCUIhm (ORCPT ); Fri, 21 Mar 2014 04:37:42 -0400 Received: from mail-pb0-f42.google.com ([209.85.160.42]:56927 "EHLO mail-pb0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753545AbaCUIhj (ORCPT ); Fri, 21 Mar 2014 04:37:39 -0400 Received: by mail-pb0-f42.google.com with SMTP id rr13so2119838pbb.15 for ; Fri, 21 Mar 2014 01:37:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=QFM3JRyqbal2nuLJX/NzGBqqKUaAnIT4f1/gSAOrkQA=; b=BA8v9WJbKkYmULWXh2vMLpz6PWfcru6q1MZvgG3ZxPGB4UHPWxWrGM1uHdoOA/pUpZ dbEmz0rjLlihnbEHrCljmAt5cP72xM7BJGikIlwD+ZBiHJ3u+RMypmJ28VaZVXfBH6pK 8MYGCWmDVLN9Nnq0xSgcGRhTPNVVLNyNQ68Ae1fO7AxfUxWY1wG8/y5C+u95qHyHj7TU Dgbs311xENzQbl0+eDwrXk5hHk4mdYPnUGPL8+Co1xOj10MpeXj42YjL7w1DOcoNSZrH btIRAC/KNqwhjkbMiZRB3Xa4bfpAcQmsGrm/GEPJJ2QQmcFS3W5Il8RgrUhtfU+PSKWm J6MA== X-Received: by 10.66.66.202 with SMTP id h10mr52351443pat.70.1395391059231; Fri, 21 Mar 2014 01:37:39 -0700 (PDT) Received: from localhost.localdomain ([115.113.119.130]) by mx.google.com with ESMTPSA id te2sm22772045pac.25.2014.03.21.01.37.34 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 21 Mar 2014 01:37:36 -0700 (PDT) From: Arun Kumar K To: linux-media@vger.kernel.org Cc: k.debski@samsung.com, s.nawrocki@samsung.com, posciak@chromium.org, arunkk.samsung@gmail.com Subject: [PATCH 3/3] [media] s5p-mfc: Don't allocate codec buffers on STREAMON. Date: Fri, 21 Mar 2014 14:07:15 +0530 Message-Id: <1395391035-27349-4-git-send-email-arun.kk@samsung.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1395391035-27349-1-git-send-email-arun.kk@samsung.com> References: <1395391035-27349-1-git-send-email-arun.kk@samsung.com> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Pawel Osciak Currently, we allocate private codec buffers on STREAMON, which may fail if we are out of memory. We don't check for failure though, which will make us crash with the codec accessing random memory. We shouldn't be failing STREAMON with out of memory errors though. So move the allocation of private codec buffers to REQBUFS for OUTPUT queue. Also, move MFC instance opening and closing to REQBUFS as well, as it's tied to allocation and deallocation of private codec buffers. Signed-off-by: Pawel Osciak Signed-off-by: Arun Kumar K --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 10 ++++----- drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 1 + drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 30 +++++++++++-------------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index 04030f5..4ee5a02 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -637,8 +637,9 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv) goto irq_cleanup_hw; case S5P_MFC_R2H_CMD_CLOSE_INSTANCE_RET: - clear_work_bit(ctx); + ctx->inst_no = MFC_NO_INSTANCE_SET; ctx->state = MFCINST_FREE; + clear_work_bit(ctx); wake_up(&ctx->queue); goto irq_cleanup_hw; @@ -758,7 +759,7 @@ static int s5p_mfc_open(struct file *file) goto err_bad_node; } ctx->fh.ctrl_handler = &ctx->ctrl_handler; - ctx->inst_no = -1; + ctx->inst_no = MFC_NO_INSTANCE_SET; /* Load firmware if this is the first instance */ if (dev->num_inst == 1) { dev->watchdog_timer.expires = jiffies + @@ -868,12 +869,11 @@ static int s5p_mfc_release(struct file *file) vb2_queue_release(&ctx->vq_dst); /* Mark context as idle */ clear_work_bit_irqsave(ctx); - /* If instance was initialised then + /* If instance was initialised and not yet freed, * return instance and free resources */ - if (ctx->inst_no != MFC_NO_INSTANCE_SET) { + if (ctx->state != MFCINST_FREE && ctx->state != MFCINST_INIT) { mfc_debug(2, "Has to free instance\n"); s5p_mfc_close_mfc_inst(dev, ctx); - ctx->inst_no = MFC_NO_INSTANCE_SET; } /* hardware locking scheme */ if (dev->curr_ctx == ctx->num) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c index ccbfcb3..865e9e0 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c @@ -461,5 +461,6 @@ void s5p_mfc_close_mfc_inst(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx) if (ctx->type == MFCINST_DECODER) s5p_mfc_hw_call(dev->mfc_ops, release_dec_desc_buffer, ctx); + ctx->inst_no = MFC_NO_INSTANCE_SET; ctx->state = MFCINST_FREE; } diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c index efc78ae..4586186 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c @@ -475,11 +475,11 @@ static int reqbufs_output(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx, ret = vb2_reqbufs(&ctx->vq_src, reqbufs); if (ret) goto out; + s5p_mfc_close_mfc_inst(dev, ctx); ctx->src_bufs_cnt = 0; + ctx->output_state = QUEUE_FREE; } else if (ctx->output_state == QUEUE_FREE) { - /* Can only request buffers after the instance - * has been opened. - */ + /* Can only request buffers when we have a valid format set. */ WARN_ON(ctx->src_bufs_cnt != 0); if (ctx->state != MFCINST_INIT) { mfc_err("Reqbufs called in an invalid state\n"); @@ -493,6 +493,13 @@ static int reqbufs_output(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx, if (ret) goto out; + ret = s5p_mfc_open_mfc_inst(dev, ctx); + if (ret) { + reqbufs->count = 0; + vb2_reqbufs(&ctx->vq_src, reqbufs); + goto out; + } + ctx->output_state = QUEUE_BUFS_REQUESTED; } else { mfc_err("Buffers have already been requested\n"); @@ -594,7 +601,7 @@ static int vidioc_querybuf(struct file *file, void *priv, return -EINVAL; } mfc_debug(2, "State: %d, buf->type: %d\n", ctx->state, buf->type); - if (ctx->state == MFCINST_INIT && + if (ctx->state == MFCINST_GOT_INST && buf->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { ret = vb2_querybuf(&ctx->vq_src, buf); } else if (ctx->state == MFCINST_RUNNING && @@ -670,24 +677,13 @@ static int vidioc_streamon(struct file *file, void *priv, enum v4l2_buf_type type) { struct s5p_mfc_ctx *ctx = fh_to_ctx(priv); - struct s5p_mfc_dev *dev = ctx->dev; int ret = -EINVAL; mfc_debug_enter(); - if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { - if (ctx->state == MFCINST_INIT) { - ctx->dst_bufs_cnt = 0; - ctx->src_bufs_cnt = 0; - ctx->capture_state = QUEUE_FREE; - ctx->output_state = QUEUE_FREE; - ret = s5p_mfc_open_mfc_inst(dev, ctx); - if (ret) - return ret; - } + if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ret = vb2_streamon(&ctx->vq_src, type); - } else if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { + else if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) ret = vb2_streamon(&ctx->vq_dst, type); - } mfc_debug_leave(); return ret; }