From patchwork Sun Dec 31 10:30:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507183 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AC58023D8 for ; Sun, 31 Dec 2023 10:31:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="h3bz7j0F" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018673; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=IdC4EJWH5PwDik2zyDs/M/lID3r33UEzF20ELGcIu6U=; b=h3bz7j0F2U3l0ejCYYiL85r3YVL1rZWXMuGBx7TLhrtEL6symaeMNGHsse+z9frWK0c7r/ Yesp7Mo3zlnu4f0pKTa1f9t/r4fyQtqHuntA3on1IT3jDEV8J5R+HexACoNaPEyLEEBXH0 /FUc4anNzAL+RI533a2v2l9rbTAUdpo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-199-lIxUDw0bN3qwIdfpvvP0cg-1; Sun, 31 Dec 2023 05:31:10 -0500 X-MC-Unique: lIxUDw0bN3qwIdfpvvP0cg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 47831831002; Sun, 31 Dec 2023 10:31:09 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id C046E492BC6; Sun, 31 Dec 2023 10:31:07 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 01/15] media: atomisp: Adjust for v4l2_subdev_state handling changes in 6.8 Date: Sun, 31 Dec 2023 11:30:43 +0100 Message-ID: <20231231103057.35837-2-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 The atomisp driver emulates a standard v4l2 device, which also works for non media-controller aware applications. Part of this requires making try_fmt calls on the sensor when a normal v4l2 app is making try_fmt calls on the /dev/video# mode. With the recent v4l2_subdev_state handling changes in 6.8 this no longer works, fixing this requires 2 changes: 1. The atomisp code was using its own internal v4l2_subdev_pad_config for this. Replace the internal v4l2_subdev_pad_config with allocating a full v4l2_subdev_state for storing the full try_fmt state. 2. The paths actually setting the fmt or crop selection now need to be passed the v4l2_subdev's active state, so that sensor drivers which are using the v4l2_subdev's active state to store their state keep working. Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_cmd.c | 58 ++++++++++-------- .../media/atomisp/pci/atomisp_internal.h | 4 +- .../staging/media/atomisp/pci/atomisp_ioctl.c | 52 +++++++++------- .../staging/media/atomisp/pci/atomisp_v4l2.c | 59 ++++++++++++++----- 4 files changed, 111 insertions(+), 62 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index 759233a7ba50..555814012fce 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -3723,12 +3723,10 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height, static int atomisp_set_crop(struct atomisp_device *isp, const struct v4l2_mbus_framefmt *format, + struct v4l2_subdev_state *sd_state, int which) { struct atomisp_input_subdev *input = &isp->inputs[isp->asd.input_curr]; - struct v4l2_subdev_state pad_state = { - .pads = &input->pad_cfg, - }; struct v4l2_subdev_selection sel = { .which = which, .target = V4L2_SEL_TGT_CROP, @@ -3754,7 +3752,7 @@ static int atomisp_set_crop(struct atomisp_device *isp, sel.r.left = ((input->native_rect.width - sel.r.width) / 2) & ~1; sel.r.top = ((input->native_rect.height - sel.r.height) / 2) & ~1; - ret = v4l2_subdev_call(input->camera, pad, set_selection, &pad_state, &sel); + ret = v4l2_subdev_call(input->camera, pad, set_selection, sd_state, &sel); if (ret) dev_err(isp->dev, "Error setting crop to %ux%u @%ux%u: %d\n", sel.r.width, sel.r.height, sel.r.left, sel.r.top, ret); @@ -3770,9 +3768,6 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f, const struct atomisp_format_bridge *fmt, *snr_fmt; struct atomisp_sub_device *asd = &isp->asd; struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr]; - struct v4l2_subdev_state pad_state = { - .pads = &input->pad_cfg, - }; struct v4l2_subdev_format format = { .which = V4L2_SUBDEV_FORMAT_TRY, }; @@ -3809,11 +3804,16 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f, dev_dbg(isp->dev, "try_mbus_fmt: asking for %ux%u\n", format.format.width, format.format.height); - ret = atomisp_set_crop(isp, &format.format, V4L2_SUBDEV_FORMAT_TRY); - if (ret) - return ret; + v4l2_subdev_lock_state(input->try_sd_state); + + ret = atomisp_set_crop(isp, &format.format, input->try_sd_state, + V4L2_SUBDEV_FORMAT_TRY); + if (ret == 0) + ret = v4l2_subdev_call(input->camera, pad, set_fmt, + input->try_sd_state, &format); + + v4l2_subdev_unlock_state(input->try_sd_state); - ret = v4l2_subdev_call(input->camera, pad, set_fmt, &pad_state, &format); if (ret) return ret; @@ -4238,9 +4238,7 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p struct atomisp_device *isp = asd->isp; struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr]; const struct atomisp_format_bridge *format; - struct v4l2_subdev_state pad_state = { - .pads = &input->pad_cfg, - }; + struct v4l2_subdev_state *act_sd_state; struct v4l2_subdev_format vformat = { .which = V4L2_SUBDEV_FORMAT_TRY, }; @@ -4268,12 +4266,18 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p /* Disable dvs if resolution can't be supported by sensor */ if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) { - ret = atomisp_set_crop(isp, &vformat.format, V4L2_SUBDEV_FORMAT_TRY); - if (ret) - return ret; + v4l2_subdev_lock_state(input->try_sd_state); + + ret = atomisp_set_crop(isp, &vformat.format, input->try_sd_state, + V4L2_SUBDEV_FORMAT_TRY); + if (ret == 0) { + vformat.which = V4L2_SUBDEV_FORMAT_TRY; + ret = v4l2_subdev_call(input->camera, pad, set_fmt, + input->try_sd_state, &vformat); + } + + v4l2_subdev_unlock_state(input->try_sd_state); - vformat.which = V4L2_SUBDEV_FORMAT_TRY; - ret = v4l2_subdev_call(input->camera, pad, set_fmt, &pad_state, &vformat); if (ret) return ret; @@ -4291,12 +4295,18 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p } } - ret = atomisp_set_crop(isp, &vformat.format, V4L2_SUBDEV_FORMAT_ACTIVE); - if (ret) - return ret; + act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera); + + ret = atomisp_set_crop(isp, &vformat.format, act_sd_state, + V4L2_SUBDEV_FORMAT_ACTIVE); + if (ret == 0) { + vformat.which = V4L2_SUBDEV_FORMAT_ACTIVE; + ret = v4l2_subdev_call(input->camera, pad, set_fmt, act_sd_state, &vformat); + } + + if (act_sd_state) + v4l2_subdev_unlock_state(act_sd_state); - vformat.which = V4L2_SUBDEV_FORMAT_ACTIVE; - ret = v4l2_subdev_call(input->camera, pad, set_fmt, NULL, &vformat); if (ret) return ret; diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h index f7b4bee9574b..d5b077e602ca 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_internal.h +++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h @@ -132,8 +132,8 @@ struct atomisp_input_subdev { /* Sensor rects for sensors which support crop */ struct v4l2_rect native_rect; struct v4l2_rect active_rect; - /* Sensor pad_cfg for which == V4L2_SUBDEV_FORMAT_TRY calls */ - struct v4l2_subdev_pad_config pad_cfg; + /* Sensor state for which == V4L2_SUBDEV_FORMAT_TRY calls */ + struct v4l2_subdev_state *try_sd_state; struct v4l2_subdev *motor; diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index 09c0091b920f..404317f6c57b 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -781,12 +781,20 @@ static int atomisp_enum_framesizes(struct file *file, void *priv, .which = V4L2_SUBDEV_FORMAT_ACTIVE, .code = input->code, }; + struct v4l2_subdev_state *act_sd_state; int ret; + if (!input->camera) + return -EINVAL; + if (input->crop_support) return atomisp_enum_framesizes_crop(isp, fsize); - ret = v4l2_subdev_call(input->camera, pad, enum_frame_size, NULL, &fse); + act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera); + ret = v4l2_subdev_call(input->camera, pad, enum_frame_size, + act_sd_state, &fse); + if (act_sd_state) + v4l2_subdev_unlock_state(act_sd_state); if (ret) return ret; @@ -803,18 +811,25 @@ static int atomisp_enum_frameintervals(struct file *file, void *priv, struct video_device *vdev = video_devdata(file); struct atomisp_device *isp = video_get_drvdata(vdev); struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd; + struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr]; struct v4l2_subdev_frame_interval_enum fie = { - .code = atomisp_in_fmt_conv[0].code, + .code = atomisp_in_fmt_conv[0].code, .index = fival->index, .width = fival->width, .height = fival->height, .which = V4L2_SUBDEV_FORMAT_ACTIVE, }; + struct v4l2_subdev_state *act_sd_state; int ret; - ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, - pad, enum_frame_interval, NULL, - &fie); + if (!input->camera) + return -EINVAL; + + act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera); + ret = v4l2_subdev_call(input->camera, pad, enum_frame_interval, + act_sd_state, &fie); + if (act_sd_state) + v4l2_subdev_unlock_state(act_sd_state); if (ret) return ret; @@ -830,30 +845,25 @@ static int atomisp_enum_fmt_cap(struct file *file, void *fh, struct video_device *vdev = video_devdata(file); struct atomisp_device *isp = video_get_drvdata(vdev); struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd; + struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr]; struct v4l2_subdev_mbus_code_enum code = { .which = V4L2_SUBDEV_FORMAT_ACTIVE, }; const struct atomisp_format_bridge *format; - struct v4l2_subdev *camera; + struct v4l2_subdev_state *act_sd_state; unsigned int i, fi = 0; - int rval; + int ret; - camera = isp->inputs[asd->input_curr].camera; - if(!camera) { - dev_err(isp->dev, "%s(): camera is NULL, device is %s\n", - __func__, vdev->name); + if (!input->camera) return -EINVAL; - } - rval = v4l2_subdev_call(camera, pad, enum_mbus_code, NULL, &code); - if (rval == -ENOIOCTLCMD) { - dev_warn(isp->dev, - "enum_mbus_code pad op not supported by %s. Please fix your sensor driver!\n", - camera->name); - } - - if (rval) - return rval; + act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera); + ret = v4l2_subdev_call(input->camera, pad, enum_mbus_code, + act_sd_state, &code); + if (act_sd_state) + v4l2_subdev_unlock_state(act_sd_state); + if (ret) + return ret; for (i = 0; i < ARRAY_SIZE(atomisp_output_fmts); i++) { format = &atomisp_output_fmts[i]; diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index c1c8501ec61f..547e1444ad97 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -862,6 +862,9 @@ static void atomisp_unregister_entities(struct atomisp_device *isp) v4l2_device_unregister(&isp->v4l2_dev); media_device_unregister(&isp->media_dev); media_device_cleanup(&isp->media_dev); + + for (i = 0; i < isp->input_cnt; i++) + __v4l2_subdev_state_free(isp->inputs[i].try_sd_state); } static int atomisp_register_entities(struct atomisp_device *isp) @@ -933,32 +936,49 @@ static int atomisp_register_entities(struct atomisp_device *isp) static void atomisp_init_sensor(struct atomisp_input_subdev *input) { + static struct lock_class_key try_sd_state_key; struct v4l2_subdev_mbus_code_enum mbus_code_enum = { }; struct v4l2_subdev_frame_size_enum fse = { }; - struct v4l2_subdev_state sd_state = { - .pads = &input->pad_cfg, - }; struct v4l2_subdev_selection sel = { }; + struct v4l2_subdev_state *try_sd_state, *act_sd_state; int i, err; + /* + * FIXME: Drivers are not supposed to use __v4l2_subdev_state_alloc() + * but atomisp needs this for try_fmt on its /dev/video# node since + * it emulates a normal v4l2 device there, passing through try_fmt / + * set_fmt to the sensor. + */ + try_sd_state = __v4l2_subdev_state_alloc(input->camera, + "atomisp:try_sd_state->lock", &try_sd_state_key); + if (IS_ERR(try_sd_state)) + return; + + input->try_sd_state = try_sd_state; + + act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera); + mbus_code_enum.which = V4L2_SUBDEV_FORMAT_ACTIVE; - err = v4l2_subdev_call(input->camera, pad, enum_mbus_code, NULL, &mbus_code_enum); + err = v4l2_subdev_call(input->camera, pad, enum_mbus_code, + act_sd_state, &mbus_code_enum); if (!err) input->code = mbus_code_enum.code; sel.which = V4L2_SUBDEV_FORMAT_ACTIVE; sel.target = V4L2_SEL_TGT_NATIVE_SIZE; - err = v4l2_subdev_call(input->camera, pad, get_selection, NULL, &sel); + err = v4l2_subdev_call(input->camera, pad, get_selection, + act_sd_state, &sel); if (err) - return; + goto unlock_act_sd_state; input->native_rect = sel.r; sel.which = V4L2_SUBDEV_FORMAT_ACTIVE; sel.target = V4L2_SEL_TGT_CROP_DEFAULT; - err = v4l2_subdev_call(input->camera, pad, get_selection, NULL, &sel); + err = v4l2_subdev_call(input->camera, pad, get_selection, + act_sd_state, &sel); if (err) - return; + goto unlock_act_sd_state; input->active_rect = sel.r; @@ -973,7 +993,8 @@ static void atomisp_init_sensor(struct atomisp_input_subdev *input) fse.code = input->code; fse.which = V4L2_SUBDEV_FORMAT_ACTIVE; - err = v4l2_subdev_call(input->camera, pad, enum_frame_size, NULL, &fse); + err = v4l2_subdev_call(input->camera, pad, enum_frame_size, + act_sd_state, &fse); if (err) break; @@ -989,22 +1010,26 @@ static void atomisp_init_sensor(struct atomisp_input_subdev *input) * for padding, set the crop rect to cover the entire sensor instead * of only the default active area. * - * Do this for both try and active formats since the try_crop rect in - * pad_cfg may influence (clamp) future try_fmt calls with which == try. + * Do this for both try and active formats since the crop rect in + * try_sd_state may influence (clamp size) in calls with which == try. */ sel.which = V4L2_SUBDEV_FORMAT_TRY; sel.target = V4L2_SEL_TGT_CROP; sel.r = input->native_rect; - err = v4l2_subdev_call(input->camera, pad, set_selection, &sd_state, &sel); + v4l2_subdev_lock_state(input->try_sd_state); + err = v4l2_subdev_call(input->camera, pad, set_selection, + input->try_sd_state, &sel); + v4l2_subdev_unlock_state(input->try_sd_state); if (err) - return; + goto unlock_act_sd_state; sel.which = V4L2_SUBDEV_FORMAT_ACTIVE; sel.target = V4L2_SEL_TGT_CROP; sel.r = input->native_rect; - err = v4l2_subdev_call(input->camera, pad, set_selection, NULL, &sel); + err = v4l2_subdev_call(input->camera, pad, set_selection, + act_sd_state, &sel); if (err) - return; + goto unlock_act_sd_state; dev_info(input->camera->dev, "Supports crop native %dx%d active %dx%d binning %d\n", input->native_rect.width, input->native_rect.height, @@ -1012,6 +1037,10 @@ static void atomisp_init_sensor(struct atomisp_input_subdev *input) input->binning_support); input->crop_support = true; + +unlock_act_sd_state: + if (act_sd_state) + v4l2_subdev_unlock_state(act_sd_state); } int atomisp_register_device_nodes(struct atomisp_device *isp) From patchwork Sun Dec 31 10:30:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507184 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7E47928F5 for ; Sun, 31 Dec 2023 10:31:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="VV8POzeO" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018675; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RRjl+EEoEb7iX2OhBiLVx7Fnpl+HQgXRWjnp9dR9eE4=; b=VV8POzeORuNWYG7f0SolEwTEF4jRNgmO4b8o5VL5f/gGyynhJiPvP1EWgmgjH1klk4Hw4q GkG32BsgKNK3m7OjfEknJgbKvCvjCZoMfNjW89TqEFv7ETLNXTWTEZFLgTMTscnI9EWv54 8fHhpIPfcCyUUIFbZ98gSeSkUZtMBJ4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-84-IX-DVSR5O16CYB9hU__i7A-1; Sun, 31 Dec 2023 05:31:11 -0500 X-MC-Unique: IX-DVSR5O16CYB9hU__i7A-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EF040185A780; Sun, 31 Dec 2023 10:31:10 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 78681492BC6; Sun, 31 Dec 2023 10:31:09 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 02/15] media: atomisp: Refactor sensor crop + fmt setting Date: Sun, 31 Dec 2023 11:30:44 +0100 Message-ID: <20231231103057.35837-3-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 There are 3 code-paths all of 3 which need to do: 1. Get try or active state 2. lock state 3. Call atomisp_set_crop() 4. Call v4l2_subdev_call(input->camera, pad, set_fmt, state, fmt) 5. unlock state Change atomisp_set_crop into atomisp_set_crop_and_fmt() which does all of this and change the 3 code-paths which need this to use the new atomisp_set_crop_and_fmt() function. Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_cmd.c | 141 +++++++----------- 1 file changed, 58 insertions(+), 83 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index 555814012fce..bfa15fb45971 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -3721,22 +3721,34 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height, *padding_h = max_t(u32, *padding_h, min_pad_h); } -static int atomisp_set_crop(struct atomisp_device *isp, - const struct v4l2_mbus_framefmt *format, - struct v4l2_subdev_state *sd_state, - int which) +static int atomisp_set_crop_and_fmt(struct atomisp_device *isp, + struct v4l2_mbus_framefmt *ffmt, + int which) { struct atomisp_input_subdev *input = &isp->inputs[isp->asd.input_curr]; struct v4l2_subdev_selection sel = { .which = which, .target = V4L2_SEL_TGT_CROP, - .r.width = format->width, - .r.height = format->height, + .r.width = ffmt->width, + .r.height = ffmt->height, }; - int ret; + struct v4l2_subdev_format format = { + .which = which, + .format = *ffmt, + }; + struct v4l2_subdev_state *sd_state; + int ret = 0; + + if (!input->camera) + return -EINVAL; + + sd_state = (which == V4L2_SUBDEV_FORMAT_TRY) ? input->try_sd_state : + input->camera->active_state; + if (sd_state) + v4l2_subdev_lock_state(sd_state); if (!input->crop_support) - return 0; + goto set_fmt; /* Cropping is done before binning, when binning double the crop rect */ if (input->binning_support && sel.r.width <= (input->native_rect.width / 2) && @@ -3757,6 +3769,14 @@ static int atomisp_set_crop(struct atomisp_device *isp, dev_err(isp->dev, "Error setting crop to %ux%u @%ux%u: %d\n", sel.r.width, sel.r.height, sel.r.left, sel.r.top, ret); +set_fmt: + if (ret == 0) + ret = v4l2_subdev_call(input->camera, pad, set_fmt, sd_state, &format); + + if (sd_state) + v4l2_subdev_unlock_state(sd_state); + + *ffmt = format.format; return ret; } @@ -3767,16 +3787,10 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f, { const struct atomisp_format_bridge *fmt, *snr_fmt; struct atomisp_sub_device *asd = &isp->asd; - struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr]; - struct v4l2_subdev_format format = { - .which = V4L2_SUBDEV_FORMAT_TRY, - }; + struct v4l2_mbus_framefmt ffmt = { }; u32 padding_w, padding_h; int ret; - if (!input->camera) - return -EINVAL; - fmt = atomisp_get_format_bridge(f->pixelformat); /* Currently, raw formats are broken!!! */ if (!fmt || fmt->sh_fmt == IA_CSS_FRAME_FORMAT_RAW) { @@ -3797,38 +3811,27 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f, * the set_fmt call, like atomisp_set_fmt_to_snr() does. */ atomisp_get_padding(isp, f->width, f->height, &padding_w, &padding_h); - v4l2_fill_mbus_format(&format.format, f, fmt->mbus_code); - format.format.width += padding_w; - format.format.height += padding_h; + v4l2_fill_mbus_format(&ffmt, f, fmt->mbus_code); + ffmt.width += padding_w; + ffmt.height += padding_h; - dev_dbg(isp->dev, "try_mbus_fmt: asking for %ux%u\n", - format.format.width, format.format.height); - - v4l2_subdev_lock_state(input->try_sd_state); - - ret = atomisp_set_crop(isp, &format.format, input->try_sd_state, - V4L2_SUBDEV_FORMAT_TRY); - if (ret == 0) - ret = v4l2_subdev_call(input->camera, pad, set_fmt, - input->try_sd_state, &format); - - v4l2_subdev_unlock_state(input->try_sd_state); + dev_dbg(isp->dev, "try_mbus_fmt: try %ux%u\n", ffmt.width, ffmt.height); + ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY); if (ret) return ret; - dev_dbg(isp->dev, "try_mbus_fmt: got %ux%u\n", - format.format.width, format.format.height); + dev_dbg(isp->dev, "try_mbus_fmt: got %ux%u\n", ffmt.width, ffmt.height); - snr_fmt = atomisp_get_format_bridge_from_mbus(format.format.code); + snr_fmt = atomisp_get_format_bridge_from_mbus(ffmt.code); if (!snr_fmt) { dev_err(isp->dev, "unknown sensor format 0x%8.8x\n", - format.format.code); + ffmt.code); return -EINVAL; } - f->width = format.format.width - padding_w; - f->height = format.format.height - padding_h; + f->width = ffmt.width - padding_w; + f->height = ffmt.height - padding_h; /* * If the format is jpeg or custom RAW, then the width and height will @@ -4236,28 +4239,22 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev); struct atomisp_sub_device *asd = pipe->asd; struct atomisp_device *isp = asd->isp; - struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr]; const struct atomisp_format_bridge *format; - struct v4l2_subdev_state *act_sd_state; - struct v4l2_subdev_format vformat = { - .which = V4L2_SUBDEV_FORMAT_TRY, - }; - struct v4l2_mbus_framefmt *ffmt = &vformat.format; - struct v4l2_mbus_framefmt *req_ffmt; + struct v4l2_mbus_framefmt req_ffmt, ffmt = { }; struct atomisp_input_stream_info *stream_info = - (struct atomisp_input_stream_info *)ffmt->reserved; + (struct atomisp_input_stream_info *)&ffmt.reserved; int ret; format = atomisp_get_format_bridge(f->pixelformat); if (!format) return -EINVAL; - v4l2_fill_mbus_format(ffmt, f, format->mbus_code); - ffmt->height += asd->sink_pad_padding_h + dvs_env_h; - ffmt->width += asd->sink_pad_padding_w + dvs_env_w; + v4l2_fill_mbus_format(&ffmt, f, format->mbus_code); + ffmt.height += asd->sink_pad_padding_h + dvs_env_h; + ffmt.width += asd->sink_pad_padding_w + dvs_env_w; dev_dbg(isp->dev, "s_mbus_fmt: ask %ux%u (padding %ux%u, dvs %ux%u)\n", - ffmt->width, ffmt->height, asd->sink_pad_padding_w, asd->sink_pad_padding_h, + ffmt.width, ffmt.height, asd->sink_pad_padding_w, asd->sink_pad_padding_h, dvs_env_w, dvs_env_h); __atomisp_init_stream_info(ATOMISP_INPUT_STREAM_GENERAL, stream_info); @@ -4266,28 +4263,17 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p /* Disable dvs if resolution can't be supported by sensor */ if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) { - v4l2_subdev_lock_state(input->try_sd_state); - - ret = atomisp_set_crop(isp, &vformat.format, input->try_sd_state, - V4L2_SUBDEV_FORMAT_TRY); - if (ret == 0) { - vformat.which = V4L2_SUBDEV_FORMAT_TRY; - ret = v4l2_subdev_call(input->camera, pad, set_fmt, - input->try_sd_state, &vformat); - } - - v4l2_subdev_unlock_state(input->try_sd_state); - + ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY); if (ret) return ret; dev_dbg(isp->dev, "video dis: sensor width: %d, height: %d\n", - ffmt->width, ffmt->height); + ffmt.width, ffmt.height); - if (ffmt->width < req_ffmt->width || - ffmt->height < req_ffmt->height) { - req_ffmt->height -= dvs_env_h; - req_ffmt->width -= dvs_env_w; + if (ffmt.width < req_ffmt.width || + ffmt.height < req_ffmt.height) { + req_ffmt.height -= dvs_env_h; + req_ffmt.width -= dvs_env_w; ffmt = req_ffmt; dev_warn(isp->dev, "can not enable video dis due to sensor limitation."); @@ -4295,32 +4281,21 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p } } - act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera); - - ret = atomisp_set_crop(isp, &vformat.format, act_sd_state, - V4L2_SUBDEV_FORMAT_ACTIVE); - if (ret == 0) { - vformat.which = V4L2_SUBDEV_FORMAT_ACTIVE; - ret = v4l2_subdev_call(input->camera, pad, set_fmt, act_sd_state, &vformat); - } - - if (act_sd_state) - v4l2_subdev_unlock_state(act_sd_state); - + ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_ACTIVE); if (ret) return ret; __atomisp_update_stream_env(asd, ATOMISP_INPUT_STREAM_GENERAL, stream_info); dev_dbg(isp->dev, "sensor width: %d, height: %d\n", - ffmt->width, ffmt->height); + ffmt.width, ffmt.height); - if (ffmt->width < ATOM_ISP_STEP_WIDTH || - ffmt->height < ATOM_ISP_STEP_HEIGHT) + if (ffmt.width < ATOM_ISP_STEP_WIDTH || + ffmt.height < ATOM_ISP_STEP_HEIGHT) return -EINVAL; if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO && - (ffmt->width < req_ffmt->width || ffmt->height < req_ffmt->height)) { + (ffmt.width < req_ffmt.width || ffmt.height < req_ffmt.height)) { dev_warn(isp->dev, "can not enable video dis due to sensor limitation."); asd->params.video_dis_en = false; @@ -4328,9 +4303,9 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p atomisp_subdev_set_ffmt(&asd->subdev, NULL, V4L2_SUBDEV_FORMAT_ACTIVE, - ATOMISP_SUBDEV_PAD_SINK, ffmt); + ATOMISP_SUBDEV_PAD_SINK, &ffmt); - return css_input_resolution_changed(asd, ffmt); + return css_input_resolution_changed(asd, &ffmt); } int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f) From patchwork Sun Dec 31 10:30:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507185 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C895133D5 for ; Sun, 31 Dec 2023 10:31:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="bRB/NhWA" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018678; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/D5kn92EyIN4VJYiMHV5JLGxxxixiL9iUiAsEf8zGsM=; b=bRB/NhWAUwDcIThPgQwbAyAcZULQICPZ0o8Zc9zK/cth/FFlEPSIzZjDnIyYTnUnTmNNCP NTrIBoPxFHodul26NValAvn50Wfqn9BrlXmzERYLMepsW65oor/JbxM4h95QOBOpP1F7vx gQeAMDK7vc1N5693GVsc8FhlpDelxcA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-605-ENkHbEGaOL-1Ps9WN0biJw-1; Sun, 31 Dec 2023 05:31:13 -0500 X-MC-Unique: ENkHbEGaOL-1Ps9WN0biJw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A3F7A833944; Sun, 31 Dec 2023 10:31:12 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2C31E492BC6; Sun, 31 Dec 2023 10:31:11 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 03/15] media: atomisp: Remove s_routing subdev call Date: Sun, 31 Dec 2023 11:30:45 +0100 Message-ID: <20231231103057.35837-4-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 sensor drivers do not implement the s_routing subdev call, so there is no use in calling it. Also the 3 0 arguments are not the arguments which a successful s_routing call is supposed to take. Signed-off-by: Hans de Goede --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index 404317f6c57b..85d0847d7e1c 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -666,14 +666,6 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input) return ret; } - /* select operating sensor */ - ret = v4l2_subdev_call(isp->inputs[input].camera, video, s_routing, - 0, 0, 0); - if (ret && (ret != -ENOIOCTLCMD)) { - dev_err(isp->dev, "Failed to select sensor\n"); - return ret; - } - if (!IS_ISP2401) { motor = isp->inputs[input].motor; } else { From patchwork Sun Dec 31 10:30:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507186 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7936033CE for ; Sun, 31 Dec 2023 10:31:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="TksYbddg" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018677; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+osCbdvrG1x7tsiXvrTIbEHYlquQIKkUtMcYRL+hNvw=; b=TksYbddgFQL101o9yNrc1ZKvq4TKSb6YTwqFfTXGhjXU8KYQbCusBFxX2Rn1PNqZ59C4wZ MjIh+hnOTvPciPyHmYSdfvhCe04FQRwqrO8e+4vDCBEPWN+uFAMK0BblKXlBf+Om6Zv6Pf CGACkqDrvmlNEQtk+7LTer6s3OhLToM= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-91-gXLTiZL7OrKGfb4rQQrBzQ-1; Sun, 31 Dec 2023 05:31:15 -0500 X-MC-Unique: gXLTiZL7OrKGfb4rQQrBzQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 58B632812FF3; Sun, 31 Dec 2023 10:31:14 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id D53B7492BC6; Sun, 31 Dec 2023 10:31:12 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 04/15] media: atomisp: Remove remaining deferred firmware loading code Date: Sun, 31 Dec 2023 11:30:46 +0100 Message-ID: <20231231103057.35837-5-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 There is a bunch of dead code left from the deferred firmware loading support which was removed in commit 8972ed6ea7a0 ("media: atomisp: Remove deferred firmware loading support"). Remove this dead code: - Remove the skip_fwload module parameter - Remove the now always NULL fw argument from ia_css_init() - Remove the fw_explicitly_loaded boolean from sh_css.c (this was always true now) - Adjust some function kernel-doc comments to match Signed-off-by: Hans de Goede --- .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- .../staging/media/atomisp/pci/atomisp_v4l2.c | 9 ----- .../media/atomisp/pci/ia_css_control.h | 29 +++++----------- .../media/atomisp/pci/ia_css_firmware.h | 6 ++-- drivers/staging/media/atomisp/pci/sh_css.c | 33 ++----------------- 5 files changed, 14 insertions(+), 65 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c index 02f06294bbfe..6fe8b0b7467a 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c @@ -757,7 +757,7 @@ int atomisp_css_init(struct atomisp_device *isp) return ret; /* Init ISP */ - err = ia_css_init(isp->dev, &isp->css_env.isp_css_env, NULL, + err = ia_css_init(isp->dev, &isp->css_env.isp_css_env, (uint32_t)mmu_base_addr, IA_CSS_IRQ_TYPE_PULSE); if (err) { dev_err(isp->dev, "css init failed --- bad firmware?\n"); diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 547e1444ad97..206fdaee5952 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -55,10 +55,6 @@ /* G-Min addition: pull this in from intel_mid_pm.h */ #define CSTATE_EXIT_LATENCY_C1 1 -static uint skip_fwload; -module_param(skip_fwload, uint, 0644); -MODULE_PARM_DESC(skip_fwload, "Skip atomisp firmware load"); - /* cross componnet debug message flag */ int dbg_level; module_param(dbg_level, int, 0644); @@ -1161,9 +1157,6 @@ atomisp_load_firmware(struct atomisp_device *isp) int rc; char *fw_path = NULL; - if (skip_fwload) - return NULL; - if (firmware_name[0] != '\0') { fw_path = firmware_name; } else { @@ -1591,8 +1584,6 @@ static void atomisp_pci_remove(struct pci_dev *pdev) atomisp_msi_irq_uninit(isp); atomisp_unregister_entities(isp); - - release_firmware(isp->firmware); } static const struct pci_device_id atomisp_pci_tbl[] = { diff --git a/drivers/staging/media/atomisp/pci/ia_css_control.h b/drivers/staging/media/atomisp/pci/ia_css_control.h index 88f031a63ba2..6a473459b346 100644 --- a/drivers/staging/media/atomisp/pci/ia_css_control.h +++ b/drivers/staging/media/atomisp/pci/ia_css_control.h @@ -30,39 +30,28 @@ * environment in which the CSS code runs. This is * used for host side memory access and message * printing. May not be NULL. - * @param[in] fw Firmware package containing the firmware for all - * predefined ISP binaries. - * if fw is NULL the firmware must be loaded before - * through a call of ia_css_load_firmware * @param[in] l1_base Base index (isp2400) * of the L1 page table. This is a physical * address or index. * @param[in] irq_type The type of interrupt to be used (edge or level) - * @return Returns -EINVAL in case of any + * @return Returns -EINVAL in case of any * errors and 0 otherwise. * * This function initializes the API which includes allocating and initializing - * internal data structures. This also interprets the firmware package. All - * contents of this firmware package are copied into local data structures, so - * the fw pointer could be freed after this function completes. + * internal data structures. + * ia_css_load_firmware() must be called to load the firmware before calling + * this function. */ int ia_css_init(struct device *dev, - const struct ia_css_env *env, - const struct ia_css_fw *fw, - u32 l1_base, - enum ia_css_irq_type irq_type); + const struct ia_css_env *env, + u32 l1_base, + enum ia_css_irq_type irq_type); /* @brief Un-initialize the CSS API. * @return None * - * This function deallocates all memory that has been allocated by the CSS API - * Exception: if you explicitly loaded firmware through ia_css_load_firmware - * you need to call ia_css_unload_firmware to deallocate the memory reserved - * for the firmware. - * After this function is called, no other CSS functions should be called - * with the exception of ia_css_init which will re-initialize the CSS code, - * ia_css_unload_firmware to unload the firmware or ia_css_load_firmware - * to load new firmware + * This function deallocates all memory that has been allocated by the CSS API. + * After this function is called, no other CSS functions should be called. */ void ia_css_uninit(void); diff --git a/drivers/staging/media/atomisp/pci/ia_css_firmware.h b/drivers/staging/media/atomisp/pci/ia_css_firmware.h index 01d2faf557cf..d3a66128b4de 100644 --- a/drivers/staging/media/atomisp/pci/ia_css_firmware.h +++ b/drivers/staging/media/atomisp/pci/ia_css_firmware.h @@ -46,10 +46,6 @@ struct device; * This function interprets the firmware package. All * contents of this firmware package are copied into local data structures, so * the fw pointer could be freed after this function completes. - * - * Rationale for this function is that it can be called before ia_css_init, and thus - * speeds up ia_css_init (ia_css_init is called each time a stream is created but the - * firmware only needs to be loaded once). */ int ia_css_load_firmware(struct device *dev, const struct ia_css_env *env, @@ -61,6 +57,8 @@ ia_css_load_firmware(struct device *dev, const struct ia_css_env *env, * This function unloads the firmware loaded by ia_css_load_firmware. * It is pointless to call this function if no firmware is loaded, * but it won't harm. Use this to deallocate all memory associated with the firmware. + * This function may only be called when the CSS API is in uninitialized state + * (e.g. after calling ia_css_uninit()). */ void ia_css_unload_firmware(void); diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c index f35c90809414..1d1fbda75da1 100644 --- a/drivers/staging/media/atomisp/pci/sh_css.c +++ b/drivers/staging/media/atomisp/pci/sh_css.c @@ -174,8 +174,6 @@ static struct sh_css_hmm_buffer_record hmm_buffer_record[MAX_HMM_BUFFER_NUM]; #define GPIO_FLASH_PIN_MASK BIT(HIVE_GPIO_STROBE_TRIGGER_PIN) -static bool fw_explicitly_loaded; - /* * Local prototypes */ @@ -1360,7 +1358,6 @@ ia_css_unload_firmware(void) ia_css_binary_uninit(); sh_css_unload_firmware(); } - fw_explicitly_loaded = false; } static void @@ -1405,13 +1402,9 @@ ia_css_load_firmware(struct device *dev, const struct ia_css_env *env, my_css.flush = env->cpu_mem_env.flush; } - ia_css_unload_firmware(); /* in case we are called twice */ err = sh_css_load_firmware(dev, fw->data, fw->bytes); - if (!err) { + if (!err) err = ia_css_binary_init_infos(); - if (!err) - fw_explicitly_loaded = true; - } ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_load_firmware() leave\n"); return err; @@ -1419,9 +1412,7 @@ ia_css_load_firmware(struct device *dev, const struct ia_css_env *env, int ia_css_init(struct device *dev, const struct ia_css_env *env, - const struct ia_css_fw *fw, - u32 mmu_l1_base, - enum ia_css_irq_type irq_type) + u32 mmu_l1_base, enum ia_css_irq_type irq_type) { int err; ia_css_spctrl_cfg spctrl_cfg; @@ -1466,8 +1457,6 @@ ia_css_init(struct device *dev, const struct ia_css_env *env, /* Check struct ia_css_init_dmem_cfg */ COMPILATION_ERROR_IF(sizeof(struct ia_css_sp_init_dmem_cfg) != SIZE_OF_IA_CSS_SP_INIT_DMEM_CFG_STRUCT); - if (!fw && !fw_explicitly_loaded) - return -EINVAL; if (!env) return -EINVAL; @@ -1543,22 +1532,7 @@ ia_css_init(struct device *dev, const struct ia_css_env *env, IA_CSS_LEAVE_ERR(err); return err; } - if (fw) { - ia_css_unload_firmware(); /* in case we already had firmware loaded */ - err = sh_css_load_firmware(dev, fw->data, fw->bytes); - if (err) { - IA_CSS_LEAVE_ERR(err); - return err; - } - err = ia_css_binary_init_infos(); - if (err) { - IA_CSS_LEAVE_ERR(err); - return err; - } - fw_explicitly_loaded = false; - my_css_save.loaded_fw = (struct ia_css_fw *)fw; - } if (!sh_css_setup_spctrl_config(&sh_css_sp_fw, SP_PROG_NAME, &spctrl_cfg)) return -EINVAL; @@ -2163,9 +2137,6 @@ ia_css_uninit(void) ifmtr_set_if_blocking_mode_reset = true; } - if (!fw_explicitly_loaded) - ia_css_unload_firmware(); - ia_css_spctrl_unload_fw(SP0_ID); sh_css_sp_set_sp_running(false); /* check and free any remaining mipi frames */ From patchwork Sun Dec 31 10:30:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507187 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3B9EB28F5 for ; Sun, 31 Dec 2023 10:31:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Ndzwlcrp" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018679; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4Qk5vDb2c2h9sLsJfdVsGU1LkgZW1c5p5JmKyi3n3Ds=; b=Ndzwlcrp4RND1Dh6U3HhtEX2QZ70vMwtDPlaNeRXokAVz8Rt716QyWAzNPiSCcNO2Ikibl qQAcwG7DEL8zf5326LZiy9qqzxGDe0nG7CnDoQkguF3X7pok3wU4xE7737GUGGcv04hzIc Fn7VKTt466W9zaxldqcHbUX9j//nYtA= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-474-79u70tKvPnuZfdgK17sEyw-1; Sun, 31 Dec 2023 05:31:16 -0500 X-MC-Unique: 79u70tKvPnuZfdgK17sEyw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 267993C0C48C; Sun, 31 Dec 2023 10:31:16 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 898EC492BC6; Sun, 31 Dec 2023 10:31:14 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 05/15] media: atomisp: Drop is_valid_device() function Date: Sun, 31 Dec 2023 11:30:47 +0100 Message-ID: <20231231103057.35837-6-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Now that a single build supports both the ISP 2400 and the ISP 2401 this function is no longer necessary. The main probe() already contains a similar switch (id->device & ATOMISP_PCI_DEVICE_SOC_MASK) checking for a known device_id. Move the revision check into the main probe() and drop the is_valid_device() function. Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_v4l2.c | 50 ++----------------- 1 file changed, 5 insertions(+), 45 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 206fdaee5952..176b39906d10 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1192,48 +1192,6 @@ atomisp_load_firmware(struct atomisp_device *isp) return fw; } -/* - * Check for flags the driver was compiled with against the PCI - * device. Always returns true on other than ISP 2400. - */ -static bool is_valid_device(struct pci_dev *pdev, const struct pci_device_id *id) -{ - const char *name; - const char *product; - - product = dmi_get_system_info(DMI_PRODUCT_NAME); - - switch (id->device & ATOMISP_PCI_DEVICE_SOC_MASK) { - case ATOMISP_PCI_DEVICE_SOC_MRFLD: - name = "Merrifield"; - break; - case ATOMISP_PCI_DEVICE_SOC_BYT: - name = "Baytrail"; - break; - case ATOMISP_PCI_DEVICE_SOC_ANN: - name = "Anniedale"; - break; - case ATOMISP_PCI_DEVICE_SOC_CHT: - name = "Cherrytrail"; - break; - default: - dev_err(&pdev->dev, "%s: unknown device ID %x04:%x04\n", - product, id->vendor, id->device); - return false; - } - - if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) { - dev_err(&pdev->dev, "%s revision %d is not unsupported\n", - name, pdev->revision); - return false; - } - - dev_info(&pdev->dev, "Detected %s version %d (ISP240%c) on %s\n", - name, pdev->revision, IS_ISP2401 ? '1' : '0', product); - - return true; -} - #define ATOM_ISP_PCI_BAR 0 static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) @@ -1244,9 +1202,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i int err, val; u32 irq; - if (!is_valid_device(pdev, id)) - return -ENODEV; - /* Pointer to struct device. */ atomisp_dev = &pdev->dev; @@ -1386,6 +1341,11 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i goto atomisp_dev_alloc_fail; } + if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) { + dev_err(&pdev->dev, "revision %d is not unsupported\n", pdev->revision); + return -ENODEV; + } + dev_info(&pdev->dev, "ISP HPLL frequency base = %d MHz\n", isp->hpll_freq); isp->max_isr_latency = ATOMISP_MAX_ISR_LATENCY; From patchwork Sun Dec 31 10:30:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507188 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 05377523C for ; Sun, 31 Dec 2023 10:31:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Z1rBS63i" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018683; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=THXKYsFKR70ZZ0iCIhtUXHKlbRr3sCXZyu8f/PhaYCM=; b=Z1rBS63i+h74fPzrRsV+MJ3fh4/bDISvEQi88QdhWf1OFHPLFnMUwDsBjowsnJ7+MA1yNr 2lWavkZ9mkSyOsJ8DnE368jeuen1V70YCcrADMhDY/A4umGpbRodHtWW6uFLlr25+KJhJ4 0HzKcaLg5+Anjgo/tE1PClKWEELwQ+o= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-679-CLw2IdNJOReBvZYROv6oIg-1; Sun, 31 Dec 2023 05:31:18 -0500 X-MC-Unique: CLw2IdNJOReBvZYROv6oIg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CF65D1C0514E; Sun, 31 Dec 2023 10:31:17 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 57B07492BC6; Sun, 31 Dec 2023 10:31:16 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 06/15] media: atomisp: Call pcim_enable_device() and pcim_iomap_regions() later Date: Sun, 31 Dec 2023 11:30:48 +0100 Message-ID: <20231231103057.35837-7-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 ATM the atomisp firmware is not available in linux-firmware, so most users will not have it installed. Move pcim_enable_device() and pcim_iomap_regions() down in atomisp_pci_probe() so that they are never called when the firmware is not there. This is a preparation patch for making the atomisp driver handle missing firmware better. Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_v4l2.c | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 176b39906d10..0eea20704e66 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1209,33 +1209,16 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i if (!pdata) dev_warn(&pdev->dev, "no platform data available\n"); - err = pcim_enable_device(pdev); - if (err) { - dev_err(&pdev->dev, "Failed to enable CI ISP device (%d)\n", err); - return err; - } - start = pci_resource_start(pdev, ATOM_ISP_PCI_BAR); dev_dbg(&pdev->dev, "start: 0x%x\n", start); - err = pcim_iomap_regions(pdev, BIT(ATOM_ISP_PCI_BAR), pci_name(pdev)); - if (err) { - dev_err(&pdev->dev, "Failed to I/O memory remapping (%d)\n", err); - goto ioremap_fail; - } - isp = devm_kzalloc(&pdev->dev, sizeof(*isp), GFP_KERNEL); - if (!isp) { - err = -ENOMEM; - goto atomisp_dev_alloc_fail; - } + if (!isp) + return -ENOMEM; isp->dev = &pdev->dev; - isp->base = pcim_iomap_table(pdev)[ATOM_ISP_PCI_BAR]; isp->saved_regs.ispmmadr = start; - dev_dbg(&pdev->dev, "atomisp mmio base: %p\n", isp->base); - mutex_init(&isp->mutex); spin_lock_init(&isp->lock); @@ -1337,8 +1320,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i break; default: dev_err(&pdev->dev, "un-supported IUNIT device\n"); - err = -ENODEV; - goto atomisp_dev_alloc_fail; + return -ENODEV; } if (pdev->revision <= ATOMISP_PCI_REV_BYT_A0_MAX) { @@ -1364,6 +1346,20 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i goto fw_validation_fail; } + err = pcim_enable_device(pdev); + if (err) { + dev_err(&pdev->dev, "Failed to enable ISP PCI device (%d)\n", err); + goto pci_enable_fail; + } + + err = pcim_iomap_regions(pdev, BIT(ATOM_ISP_PCI_BAR), pci_name(pdev)); + if (err) { + dev_err(&pdev->dev, "Failed to I/O memory remapping (%d)\n", err); + goto ioremap_fail; + } + + isp->base = pcim_iomap_table(pdev)[ATOM_ISP_PCI_BAR]; + pci_set_master(pdev); err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI); @@ -1495,6 +1491,9 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i atomisp_msi_irq_uninit(isp); pci_free_irq_vectors(pdev); enable_msi_fail: + pcim_iounmap_regions(pdev, BIT(ATOM_ISP_PCI_BAR)); +ioremap_fail: +pci_enable_fail: fw_validation_fail: release_firmware(isp->firmware); load_fw_fail: @@ -1519,10 +1518,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power(isp, false)) dev_err(&pdev->dev, "Failed to switch off ISP\n"); -atomisp_dev_alloc_fail: - pcim_iounmap_regions(pdev, BIT(ATOM_ISP_PCI_BAR)); - -ioremap_fail: return err; } From patchwork Sun Dec 31 10:30:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507190 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 135EE33D5 for ; Sun, 31 Dec 2023 10:31:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="bsWL9AaG" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018687; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8roqVBsjH2VTw0PdNh5kieuqyXZ5Eq9sxg65swMshRw=; b=bsWL9AaGxBpylkq5Amb6eJsMOZOKoMDtazgi3LCnPo3QXH0P/IJUavJ3IcFNqLHylWOC8I CU8IJHT1lCY6wQLLrNdsxnoTIo/VZF+3y8LCIEw9ASb557A5SPhD2wGlK/OoASJmRjPHn3 GtT/+Md1QRlbMdUZWPgM6TdEr0xFFLk= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-687-rb4PMvagPKiKCjF5_2m7aA-1; Sun, 31 Dec 2023 05:31:20 -0500 X-MC-Unique: rb4PMvagPKiKCjF5_2m7aA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D1F511C0514E; Sun, 31 Dec 2023 10:31:19 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 59979492BC6; Sun, 31 Dec 2023 10:31:18 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 07/15] media: atomisp: Fix probe error-exit path Date: Sun, 31 Dec 2023 11:30:49 +0100 Message-ID: <20231231103057.35837-8-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Fix probe error-exit path: -Add a missing ia_css_unload_firmware() call when v4l2_async_nf_register() fails -Add a missing pm_runtime_forbid() call to undo the pm_runtime_allow() call -Remove the unnecessary pcim_iounmap_regions() those are devm managed so they will get cleaned up automatically on an error exit from probe() -Rename the labels to avoid having multiple labels pointing to the same place in the error-exit path Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_v4l2.c | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 0eea20704e66..336c5a895ecc 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1337,25 +1337,25 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i if (!isp->firmware) { err = -ENOENT; dev_dbg(&pdev->dev, "Firmware load failed\n"); - goto load_fw_fail; + goto error_power_off; } err = sh_css_check_firmware_version(isp->dev, isp->firmware->data); if (err) { dev_dbg(&pdev->dev, "Firmware version check failed\n"); - goto fw_validation_fail; + goto error_release_firmware; } err = pcim_enable_device(pdev); if (err) { dev_err(&pdev->dev, "Failed to enable ISP PCI device (%d)\n", err); - goto pci_enable_fail; + goto error_release_firmware; } err = pcim_iomap_regions(pdev, BIT(ATOM_ISP_PCI_BAR), pci_name(pdev)); if (err) { dev_err(&pdev->dev, "Failed to I/O memory remapping (%d)\n", err); - goto ioremap_fail; + goto error_release_firmware; } isp->base = pcim_iomap_table(pdev)[ATOM_ISP_PCI_BAR]; @@ -1365,7 +1365,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI); if (err < 0) { dev_err(&pdev->dev, "Failed to enable msi (%d)\n", err); - goto enable_msi_fail; + goto error_release_firmware; } atomisp_msi_irq_init(isp); @@ -1408,13 +1408,13 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i err = atomisp_initialize_modules(isp); if (err < 0) { dev_err(&pdev->dev, "atomisp_initialize_modules (%d)\n", err); - goto initialize_modules_fail; + goto error_irq_uninit; } err = atomisp_register_entities(isp); if (err < 0) { dev_err(&pdev->dev, "atomisp_register_entities failed (%d)\n", err); - goto register_entities_fail; + goto error_uninitialize_modules; } INIT_WORK(&isp->assert_recovery_work, atomisp_assert_recovery_work); @@ -1453,14 +1453,14 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i IRQF_SHARED, "isp_irq", isp); if (err) { dev_err(&pdev->dev, "Failed to request irq (%d)\n", err); - goto request_irq_fail; + goto error_unregister_entities; } /* Load firmware into ISP memory */ err = atomisp_css_load_firmware(isp); if (err) { dev_err(&pdev->dev, "Failed to init css.\n"); - goto css_init_fail; + goto error_free_irq; } /* Clear FW image from memory */ release_firmware(isp->firmware); @@ -1470,33 +1470,32 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i err = v4l2_async_nf_register(&isp->notifier); if (err) { dev_err(isp->dev, "failed to register async notifier : %d\n", err); - goto css_init_fail; + goto error_unload_firmware; } atomisp_drvfs_init(isp); return 0; -css_init_fail: +error_unload_firmware: + ia_css_unload_firmware(); +error_free_irq: devm_free_irq(&pdev->dev, pdev->irq, isp); -request_irq_fail: +error_unregister_entities: hmm_cleanup(); + pm_runtime_forbid(&pdev->dev); pm_runtime_get_noresume(&pdev->dev); dev_pm_domain_set(&pdev->dev, NULL); atomisp_unregister_entities(isp); -register_entities_fail: +error_uninitialize_modules: atomisp_uninitialize_modules(isp); -initialize_modules_fail: +error_irq_uninit: cpu_latency_qos_remove_request(&isp->pm_qos); atomisp_msi_irq_uninit(isp); pci_free_irq_vectors(pdev); -enable_msi_fail: - pcim_iounmap_regions(pdev, BIT(ATOM_ISP_PCI_BAR)); -ioremap_fail: -pci_enable_fail: -fw_validation_fail: +error_release_firmware: release_firmware(isp->firmware); -load_fw_fail: +error_power_off: /* * Switch off ISP, as keeping it powered on would prevent * reaching S0ix states. From patchwork Sun Dec 31 10:30:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507189 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4F909322A for ; Sun, 31 Dec 2023 10:31:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="gk2L7dMN" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018685; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bVNw4HygPHIpPoa8ZE/baFU/gyqzrIuRJMtfWwnNS+Y=; b=gk2L7dMN7NFQ5vb5em6JpK0MvFfNdd/S34ddMHVOHCYngLWwN0pXdPNBwYJXzLrd0EVuJw ewnPa5ERMmt59pVQyzZAbdDecbxDDzBf73wpcC1fu5dLE8GDy9kjxDsHVU2uE95eFeY51x 7AciiRuXOk9YpU8EDpFWM1ACIAKCKhU= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-34-xtljsz9-NBClt2aFPLLIdA-1; Sun, 31 Dec 2023 05:31:22 -0500 X-MC-Unique: xtljsz9-NBClt2aFPLLIdA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 84E223C0C48C; Sun, 31 Dec 2023 10:31:21 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0EEA1492BC6; Sun, 31 Dec 2023 10:31:19 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 08/15] media: atomisp: Fix atomisp_pci_remove() Date: Sun, 31 Dec 2023 11:30:50 +0100 Message-ID: <20231231103057.35837-9-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Fix atomisp_pci_remove(): -Remove uninformative "Removing atomisp driver" log message -Add missing devm_free_irq(), atomisp_uninitialize_modules() and pci_free_irq_vectors() calls -Move atomisp_msi_irq_uninit() down so that the remove() order is an exact mirror of the probe() order Signed-off-by: Hans de Goede --- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 336c5a895ecc..f3bd2c03dea5 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1524,11 +1524,10 @@ static void atomisp_pci_remove(struct pci_dev *pdev) { struct atomisp_device *isp = pci_get_drvdata(pdev); - dev_info(&pdev->dev, "Removing atomisp driver\n"); - atomisp_drvfs_exit(); ia_css_unload_firmware(); + devm_free_irq(&pdev->dev, pdev->irq, isp); hmm_cleanup(); pm_runtime_forbid(&pdev->dev); @@ -1536,8 +1535,10 @@ static void atomisp_pci_remove(struct pci_dev *pdev) dev_pm_domain_set(&pdev->dev, NULL); cpu_latency_qos_remove_request(&isp->pm_qos); - atomisp_msi_irq_uninit(isp); atomisp_unregister_entities(isp); + atomisp_uninitialize_modules(isp); + atomisp_msi_irq_uninit(isp); + pci_free_irq_vectors(pdev); } static const struct pci_device_id atomisp_pci_tbl[] = { From patchwork Sun Dec 31 10:30:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507191 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 64447539F for ; Sun, 31 Dec 2023 10:31:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="YB43SHhI" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018689; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4edGzpRnZq7kVgd+JBN48IeCWGiNpHq7TZgEGSopufs=; b=YB43SHhIFrujeDHah/4zmLfzvddvb65U39gzB0xnZJNqOdb2amVKFj32jT3NcH5eBL1lnp Dt0CPWcfxd7WWpbiH+ZCfCK86YayuQZ+XMWIXbeOp7MuoCaPonBy7bo+Kw6ORHv/wudzfJ dMrV+5j2oJNSqbavc0xhL07wlUa0cVM= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-284-7kQ5xAdrPKerHCYNRsQM7g-1; Sun, 31 Dec 2023 05:31:23 -0500 X-MC-Unique: 7kQ5xAdrPKerHCYNRsQM7g-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 39696831014; Sun, 31 Dec 2023 10:31:23 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id B5D4A492BC6; Sun, 31 Dec 2023 10:31:21 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 09/15] media: atomisp: Group cpu_latency_qos_add_request() call together with other PM calls Date: Sun, 31 Dec 2023 11:30:51 +0100 Message-ID: <20231231103057.35837-10-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Group the cpu_latency_qos_add_request() call in probe() together with the other PM calls in probe(). This is a preparation patch for futher PM fixes / work. Signed-off-by: Hans de Goede --- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index f3bd2c03dea5..7d99b53107b0 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1370,8 +1370,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i atomisp_msi_irq_init(isp); - cpu_latency_qos_add_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE); - /* * for MRFLD, Software/firmware needs to write a 1 to bit 0 of * the register at CSI_RECEIVER_SELECTION_REG to enable SH CSI @@ -1440,6 +1438,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i isp->pm_domain.ops.suspend = atomisp_suspend; isp->pm_domain.ops.resume = atomisp_resume; + cpu_latency_qos_add_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE); dev_pm_domain_set(&pdev->dev, &isp->pm_domain); pm_runtime_put_noidle(&pdev->dev); @@ -1486,11 +1485,11 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i pm_runtime_forbid(&pdev->dev); pm_runtime_get_noresume(&pdev->dev); dev_pm_domain_set(&pdev->dev, NULL); + cpu_latency_qos_remove_request(&isp->pm_qos); atomisp_unregister_entities(isp); error_uninitialize_modules: atomisp_uninitialize_modules(isp); error_irq_uninit: - cpu_latency_qos_remove_request(&isp->pm_qos); atomisp_msi_irq_uninit(isp); pci_free_irq_vectors(pdev); error_release_firmware: From patchwork Sun Dec 31 10:30:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507193 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1515033D5 for ; Sun, 31 Dec 2023 10:31:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="CPcpLMSP" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018691; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=50IKayWWLqHw5VuwhWK+OJNcSeKcRRhBGrNjS+iJJnw=; b=CPcpLMSPj+7U5fiE5536qFdH9EQpui6/9tiQn81tMblD7+1Hkqv1Zx06VO4M1nwLROpRbM q09rrxtEntfsxzEATjT72TDjm+HAJAObR9JiGa+ily0J4x8IJZ3g+6X75cEIW12Au7oCz+ +zyepq64X1G0YiZNlr/T/KNI+NiG1GY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-183-ak6lnlX5Phuy7qLB1LNirw-1; Sun, 31 Dec 2023 05:31:25 -0500 X-MC-Unique: ak6lnlX5Phuy7qLB1LNirw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E0E19832D1A; Sun, 31 Dec 2023 10:31:24 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6A8FE492BC6; Sun, 31 Dec 2023 10:31:23 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 10/15] media: atomisp: Fix probe()/remove() power-management Date: Sun, 31 Dec 2023 11:30:52 +0100 Message-ID: <20231231103057.35837-11-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Fix probe()/remove() power-management: -Currently the driver uses pm_runtime_put_noidle() and relies on userspace to open + close the /dev/video# node at least once to actually turn the ISP off. Replace the pm_runtime_put_noidle() with pm_runtime_put_sync() to make sure that the device is turned off without relying on userspace for this. This also ensures that atomisp_css_init() is run (by atomisp_power_on()) if the first userspace process opening /dev/video# wants to do more then just query the v4l2-caps. As part of this change move the pm setup code in probe() to just before calling v4l2_async_nf_register() which registers the /dev/* nodes, so that the device is left on for the entirety of the probe() function. -Remove the turning off of the atomisp from the exit-error path, the PCI subsystem and subsequent probe() attempts expect the device to be in the on state when probe() fails. This also fixes the atomisp driver causing the system to hang / freeze when its firmware is missing. This freeze is caused by an unidentified bug in the power-off on exit-error code which is now removed. -Make sure that remove() properly powers on the device by replacing pm_runtime_get_noresume() with pm_runtime_get_sync. The PCI subsystem and subsequent probe() attempts expect the device to be in the on state after unbinding the driver. -Note this also swaps the order of put()/allow() and forbid()/get() so that the sync versions actually work by calling allow() before put() and forbid() after get() Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_v4l2.c | 98 +++++++------------ 1 file changed, 37 insertions(+), 61 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 7d99b53107b0..6e8c9add35f9 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1200,7 +1200,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i struct atomisp_device *isp; unsigned int start; int err, val; - u32 irq; /* Pointer to struct device. */ atomisp_dev = &pdev->dev; @@ -1334,11 +1333,8 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i /* Load isp firmware from user space */ isp->firmware = atomisp_load_firmware(isp); - if (!isp->firmware) { - err = -ENOENT; - dev_dbg(&pdev->dev, "Firmware load failed\n"); - goto error_power_off; - } + if (!isp->firmware) + return -ENOENT; err = sh_css_check_firmware_version(isp->dev, isp->firmware->data); if (err) { @@ -1420,30 +1416,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i /* save the iunit context only once after all the values are init'ed. */ atomisp_save_iunit_reg(isp); - /* - * The atomisp does not use standard PCI power-management through the - * PCI config space. Instead this driver directly tells the P-Unit to - * disable the ISP over the IOSF. The standard PCI subsystem pm_ops will - * try to access the config space before (resume) / after (suspend) this - * driver has turned the ISP on / off, resulting in the following errors: - * - * "Unable to change power state from D0 to D3hot, device inaccessible" - * "Unable to change power state from D3cold to D0, device inaccessible" - * - * To avoid these errors override the pm_domain so that all the PCI - * subsys suspend / resume handling is skipped. - */ - isp->pm_domain.ops.runtime_suspend = atomisp_power_off; - isp->pm_domain.ops.runtime_resume = atomisp_power_on; - isp->pm_domain.ops.suspend = atomisp_suspend; - isp->pm_domain.ops.resume = atomisp_resume; - - cpu_latency_qos_add_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE); - dev_pm_domain_set(&pdev->dev, &isp->pm_domain); - - pm_runtime_put_noidle(&pdev->dev); - pm_runtime_allow(&pdev->dev); - /* Init ISP memory management */ hmm_init(); @@ -1466,6 +1438,30 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i isp->firmware = NULL; isp->css_env.isp_css_fw.data = NULL; + /* + * The atomisp does not use standard PCI power-management through the + * PCI config space. Instead this driver directly tells the P-Unit to + * disable the ISP over the IOSF. The standard PCI subsystem pm_ops will + * try to access the config space before (resume) / after (suspend) this + * driver has turned the ISP on / off, resulting in the following errors: + * + * "Unable to change power state from D0 to D3hot, device inaccessible" + * "Unable to change power state from D3cold to D0, device inaccessible" + * + * To avoid these errors override the pm_domain so that all the PCI + * subsys suspend / resume handling is skipped. + */ + isp->pm_domain.ops.runtime_suspend = atomisp_power_off; + isp->pm_domain.ops.runtime_resume = atomisp_power_on; + isp->pm_domain.ops.suspend = atomisp_suspend; + isp->pm_domain.ops.resume = atomisp_resume; + + cpu_latency_qos_add_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE); + dev_pm_domain_set(&pdev->dev, &isp->pm_domain); + + pm_runtime_allow(&pdev->dev); + pm_runtime_put_sync_suspend(&pdev->dev); + err = v4l2_async_nf_register(&isp->notifier); if (err) { dev_err(isp->dev, "failed to register async notifier : %d\n", err); @@ -1477,15 +1473,15 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i return 0; error_unload_firmware: + pm_runtime_get_sync(&pdev->dev); + pm_runtime_forbid(&pdev->dev); + dev_pm_domain_set(&pdev->dev, NULL); + cpu_latency_qos_remove_request(&isp->pm_qos); ia_css_unload_firmware(); error_free_irq: devm_free_irq(&pdev->dev, pdev->irq, isp); error_unregister_entities: hmm_cleanup(); - pm_runtime_forbid(&pdev->dev); - pm_runtime_get_noresume(&pdev->dev); - dev_pm_domain_set(&pdev->dev, NULL); - cpu_latency_qos_remove_request(&isp->pm_qos); atomisp_unregister_entities(isp); error_uninitialize_modules: atomisp_uninitialize_modules(isp); @@ -1494,28 +1490,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i pci_free_irq_vectors(pdev); error_release_firmware: release_firmware(isp->firmware); -error_power_off: - /* - * Switch off ISP, as keeping it powered on would prevent - * reaching S0ix states. - * - * The following lines have been copied from atomisp suspend path - */ - - pci_read_config_dword(pdev, PCI_INTERRUPT_CTRL, &irq); - irq &= BIT(INTR_IIR); - pci_write_config_dword(pdev, PCI_INTERRUPT_CTRL, irq); - - pci_read_config_dword(pdev, PCI_INTERRUPT_CTRL, &irq); - irq &= ~BIT(INTR_IER); - pci_write_config_dword(pdev, PCI_INTERRUPT_CTRL, irq); - - atomisp_msi_irq_uninit(isp); - - /* Address later when we worry about the ...field chips */ - if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power(isp, false)) - dev_err(&pdev->dev, "Failed to switch off ISP\n"); - return err; } @@ -1525,15 +1499,17 @@ static void atomisp_pci_remove(struct pci_dev *pdev) atomisp_drvfs_exit(); + pm_runtime_get_sync(&pdev->dev); + pm_runtime_forbid(&pdev->dev); + dev_pm_domain_set(&pdev->dev, NULL); + cpu_latency_qos_remove_request(&isp->pm_qos); + + /* Undo ia_css_init() from atomisp_power_on() */ + atomisp_css_uninit(isp); ia_css_unload_firmware(); devm_free_irq(&pdev->dev, pdev->irq, isp); hmm_cleanup(); - pm_runtime_forbid(&pdev->dev); - pm_runtime_get_noresume(&pdev->dev); - dev_pm_domain_set(&pdev->dev, NULL); - cpu_latency_qos_remove_request(&isp->pm_qos); - atomisp_unregister_entities(isp); atomisp_uninitialize_modules(isp); atomisp_msi_irq_uninit(isp); From patchwork Sun Dec 31 10:30:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507192 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8525A566D for ; Sun, 31 Dec 2023 10:31:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="jNmW28IT" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018690; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hqcqn3wVFqcuHnInqOuIW31KNg+Ka5D/Uw9Vs/4vzU0=; b=jNmW28ITqbs7FUdKY7XFxU+2wRAX5BUv3427xVFBnHTXaIbwo1kSHPU6laDZYpE4gITfAF ERYRWFLx3r1lUEnuj1rTo+4BHMSZuxv4g3xKubFQJOwYNY4KPaF/vNXgg59qb5ZBg/6eLJ Xy3/pE1QikDJjC8rZg2qUCDzzjaeWoM= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-659--f0QRKQEOnat25G1pLQ1lA-1; Sun, 31 Dec 2023 05:31:27 -0500 X-MC-Unique: -f0QRKQEOnat25G1pLQ1lA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 965E0101A52A; Sun, 31 Dec 2023 10:31:26 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1D43E492BC6; Sun, 31 Dec 2023 10:31:25 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 11/15] media: atomisp: Replace atomisp_drvfs attr with using driver.dev_groups attr Date: Sun, 31 Dec 2023 11:30:53 +0100 Message-ID: <20231231103057.35837-12-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 sysfs attributes preferably should not be manually be registered but instead the driver.groups / driver.dev_groups driver struct members should be used to have the driver core handle this in a race free manner. Using driver.groups would be the most direct replacement for driver_[add|remove]_file, but some of the attributes actually need access to the struct atomisp_device (*), so as part of modernizing this part of the atomisp driver this change also makes the sysfs attribute device attributes instead of driver attributes. *) Before this change accessing these attributes without the driver having bound would result in a NULL pointer deref, this commit fixes this. Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_drvfs.c | 148 +++++++----------- .../staging/media/atomisp/pci/atomisp_drvfs.h | 5 +- .../staging/media/atomisp/pci/atomisp_v4l2.c | 9 +- 3 files changed, 62 insertions(+), 100 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c index 1df534bf54d3..293171da1266 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.c +++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.c @@ -27,31 +27,17 @@ #include "hmm/hmm.h" #include "ia_css_debug.h" +#define OPTION_BIN_LIST BIT(0) +#define OPTION_BIN_RUN BIT(1) +#define OPTION_VALID (OPTION_BIN_LIST | OPTION_BIN_RUN) + /* - * _iunit_debug: - * dbglvl: iunit css driver trace level * dbgopt: iunit debug option: * bit 0: binary list * bit 1: running binary * bit 2: memory statistic -*/ -struct _iunit_debug { - struct device_driver *drv; - struct atomisp_device *isp; - unsigned int dbglvl; - unsigned int dbgfun; - unsigned int dbgopt; -}; - -#define OPTION_BIN_LIST BIT(0) -#define OPTION_BIN_RUN BIT(1) -#define OPTION_VALID (OPTION_BIN_LIST \ - | OPTION_BIN_RUN) - -static struct _iunit_debug iunit_debug = { - .dbglvl = 0, - .dbgopt = OPTION_BIN_LIST, -}; + */ +unsigned int dbgopt = OPTION_BIN_LIST; static inline int iunit_dump_dbgopt(struct atomisp_device *isp, unsigned int opt) @@ -88,34 +74,44 @@ static inline int iunit_dump_dbgopt(struct atomisp_device *isp, return ret; } -static ssize_t iunit_dbglvl_show(struct device_driver *drv, char *buf) +static ssize_t dbglvl_show(struct device *dev, struct device_attribute *attr, + char *buf) { - iunit_debug.dbglvl = dbg_level; - return sysfs_emit(buf, "dtrace level:%u\n", iunit_debug.dbglvl); + unsigned int dbglvl = ia_css_debug_get_dtrace_level(); + + return sysfs_emit(buf, "dtrace level:%u\n", dbglvl); } -static ssize_t iunit_dbglvl_store(struct device_driver *drv, const char *buf, - size_t size) +static ssize_t dbglvl_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t size) { - if (kstrtouint(buf, 10, &iunit_debug.dbglvl) - || iunit_debug.dbglvl < 1 - || iunit_debug.dbglvl > 9) { + unsigned int dbglvl; + int ret; + + ret = kstrtouint(buf, 10, &dbglvl); + if (ret) + return ret; + + if (dbglvl < 1 || dbglvl > 9) return -ERANGE; - } - ia_css_debug_set_dtrace_level(iunit_debug.dbglvl); + ia_css_debug_set_dtrace_level(dbglvl); return size; } +static DEVICE_ATTR_RW(dbglvl); -static ssize_t iunit_dbgfun_show(struct device_driver *drv, char *buf) +static ssize_t dbgfun_show(struct device *dev, struct device_attribute *attr, + char *buf) { - iunit_debug.dbgfun = atomisp_get_css_dbgfunc(); - return sysfs_emit(buf, "dbgfun opt:%u\n", iunit_debug.dbgfun); + unsigned int dbgfun = atomisp_get_css_dbgfunc(); + + return sysfs_emit(buf, "dbgfun opt:%u\n", dbgfun); } -static ssize_t iunit_dbgfun_store(struct device_driver *drv, const char *buf, - size_t size) +static ssize_t dbgfun_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t size) { + struct atomisp_device *isp = dev_get_drvdata(dev); unsigned int opt; int ret; @@ -123,23 +119,20 @@ static ssize_t iunit_dbgfun_store(struct device_driver *drv, const char *buf, if (ret) return ret; - ret = atomisp_set_css_dbgfunc(iunit_debug.isp, opt); - if (ret) - return ret; + return atomisp_set_css_dbgfunc(isp, opt); +} +static DEVICE_ATTR_RW(dbgfun); - iunit_debug.dbgfun = opt; - - return size; +static ssize_t dbgopt_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "option:0x%x\n", dbgopt); } -static ssize_t iunit_dbgopt_show(struct device_driver *drv, char *buf) -{ - return sysfs_emit(buf, "option:0x%x\n", iunit_debug.dbgopt); -} - -static ssize_t iunit_dbgopt_store(struct device_driver *drv, const char *buf, - size_t size) +static ssize_t dbgopt_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t size) { + struct atomisp_device *isp = dev_get_drvdata(dev); unsigned int opt; int ret; @@ -147,56 +140,27 @@ static ssize_t iunit_dbgopt_store(struct device_driver *drv, const char *buf, if (ret) return ret; - iunit_debug.dbgopt = opt; - ret = iunit_dump_dbgopt(iunit_debug.isp, iunit_debug.dbgopt); + dbgopt = opt; + ret = iunit_dump_dbgopt(isp, dbgopt); if (ret) return ret; return size; } +static DEVICE_ATTR_RW(dbgopt); -static const struct driver_attribute iunit_drvfs_attrs[] = { - __ATTR(dbglvl, 0644, iunit_dbglvl_show, iunit_dbglvl_store), - __ATTR(dbgfun, 0644, iunit_dbgfun_show, iunit_dbgfun_store), - __ATTR(dbgopt, 0644, iunit_dbgopt_show, iunit_dbgopt_store), +static struct attribute *dbg_attrs[] = { + &dev_attr_dbglvl.attr, + &dev_attr_dbgfun.attr, + &dev_attr_dbgopt.attr, + NULL }; -static int iunit_drvfs_create_files(struct device_driver *drv) -{ - int i, ret = 0; +static const struct attribute_group dbg_attr_group = { + .attrs = dbg_attrs, +}; - for (i = 0; i < ARRAY_SIZE(iunit_drvfs_attrs); i++) - ret |= driver_create_file(drv, &iunit_drvfs_attrs[i]); - - return ret; -} - -static void iunit_drvfs_remove_files(struct device_driver *drv) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(iunit_drvfs_attrs); i++) - driver_remove_file(drv, &iunit_drvfs_attrs[i]); -} - -int atomisp_drvfs_init(struct atomisp_device *isp) -{ - struct device_driver *drv = isp->dev->driver; - int ret; - - iunit_debug.isp = isp; - iunit_debug.drv = drv; - - ret = iunit_drvfs_create_files(iunit_debug.drv); - if (ret) { - dev_err(isp->dev, "drvfs_create_files error: %d\n", ret); - iunit_drvfs_remove_files(iunit_debug.drv); - } - - return ret; -} - -void atomisp_drvfs_exit(void) -{ - iunit_drvfs_remove_files(iunit_debug.drv); -} +const struct attribute_group *dbg_attr_groups[] = { + &dbg_attr_group, + NULL +}; diff --git a/drivers/staging/media/atomisp/pci/atomisp_drvfs.h b/drivers/staging/media/atomisp/pci/atomisp_drvfs.h index 8f4cc722b881..8495cc133c06 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_drvfs.h +++ b/drivers/staging/media/atomisp/pci/atomisp_drvfs.h @@ -19,7 +19,8 @@ #ifndef __ATOMISP_DRVFS_H__ #define __ATOMISP_DRVFS_H__ -int atomisp_drvfs_init(struct atomisp_device *isp); -void atomisp_drvfs_exit(void); +#include + +extern const struct attribute_group *dbg_attr_groups[]; #endif /* __ATOMISP_DRVFS_H__ */ diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 6e8c9add35f9..0c78c1d82659 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1468,8 +1468,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i goto error_unload_firmware; } - atomisp_drvfs_init(isp); - return 0; error_unload_firmware: @@ -1497,8 +1495,6 @@ static void atomisp_pci_remove(struct pci_dev *pdev) { struct atomisp_device *isp = pci_get_drvdata(pdev); - atomisp_drvfs_exit(); - pm_runtime_get_sync(&pdev->dev); pm_runtime_forbid(&pdev->dev); dev_pm_domain_set(&pdev->dev, NULL); @@ -1529,11 +1525,12 @@ static const struct pci_device_id atomisp_pci_tbl[] = { {PCI_DEVICE(PCI_VENDOR_ID_INTEL, ATOMISP_PCI_DEVICE_SOC_CHT)}, {0,} }; - MODULE_DEVICE_TABLE(pci, atomisp_pci_tbl); - static struct pci_driver atomisp_pci_driver = { + .driver = { + .dev_groups = dbg_attr_groups, + }, .name = "atomisp-isp2", .id_table = atomisp_pci_tbl, .probe = atomisp_pci_probe, From patchwork Sun Dec 31 10:30:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507194 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8683053A8 for ; Sun, 31 Dec 2023 10:31:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="PouY1al9" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018692; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Jdxf0apCEx6ROxq+hFkMBohmP/+BdrI6zj4C+8lCkwI=; b=PouY1al9DTeOJHKVSZaVfP97xsM+ibAy0j/iXLWh6lMac+IteH3pT/0X3VN5gjyVgtsZNS 1jDWNW+W9ePyzaBAD8J1M/BlRfgCRGc7pw/+1DW4VC0uIxPugZy7/beyxj3HRrWyd6CObS GDSsC10Jul+N61ej4yJw/FtEs55OG+4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-643-PuUZVprCPTmp56Ptpke6tg-1; Sun, 31 Dec 2023 05:31:28 -0500 X-MC-Unique: PuUZVprCPTmp56Ptpke6tg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 49975185A781; Sun, 31 Dec 2023 10:31:28 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id C7122492BC6; Sun, 31 Dec 2023 10:31:26 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 12/15] media: atomisp: Move power-management [un]init into atomisp_pm_[un]init() Date: Sun, 31 Dec 2023 11:30:54 +0100 Message-ID: <20231231103057.35837-13-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Move the power-management setup and cleanup code into atomisp_pm_[un]init() helper functions. Signed-off-by: Hans de Goede --- .../staging/media/atomisp/pci/atomisp_v4l2.c | 69 ++++++++++--------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 0c78c1d82659..f44be2d656a8 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -1192,6 +1192,41 @@ atomisp_load_firmware(struct atomisp_device *isp) return fw; } +static void atomisp_pm_init(struct atomisp_device *isp) +{ + /* + * The atomisp does not use standard PCI power-management through the + * PCI config space. Instead this driver directly tells the P-Unit to + * disable the ISP over the IOSF. The standard PCI subsystem pm_ops will + * try to access the config space before (resume) / after (suspend) this + * driver has turned the ISP on / off, resulting in the following errors: + * + * "Unable to change power state from D0 to D3hot, device inaccessible" + * "Unable to change power state from D3cold to D0, device inaccessible" + * + * To avoid these errors override the pm_domain so that all the PCI + * subsys suspend / resume handling is skipped. + */ + isp->pm_domain.ops.runtime_suspend = atomisp_power_off; + isp->pm_domain.ops.runtime_resume = atomisp_power_on; + isp->pm_domain.ops.suspend = atomisp_suspend; + isp->pm_domain.ops.resume = atomisp_resume; + + cpu_latency_qos_add_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE); + dev_pm_domain_set(isp->dev, &isp->pm_domain); + + pm_runtime_allow(isp->dev); + pm_runtime_put_sync_suspend(isp->dev); +} + +static void atomisp_pm_uninit(struct atomisp_device *isp) +{ + pm_runtime_get_sync(isp->dev); + pm_runtime_forbid(isp->dev); + dev_pm_domain_set(isp->dev, NULL); + cpu_latency_qos_remove_request(&isp->pm_qos); +} + #define ATOM_ISP_PCI_BAR 0 static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) @@ -1438,29 +1473,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i isp->firmware = NULL; isp->css_env.isp_css_fw.data = NULL; - /* - * The atomisp does not use standard PCI power-management through the - * PCI config space. Instead this driver directly tells the P-Unit to - * disable the ISP over the IOSF. The standard PCI subsystem pm_ops will - * try to access the config space before (resume) / after (suspend) this - * driver has turned the ISP on / off, resulting in the following errors: - * - * "Unable to change power state from D0 to D3hot, device inaccessible" - * "Unable to change power state from D3cold to D0, device inaccessible" - * - * To avoid these errors override the pm_domain so that all the PCI - * subsys suspend / resume handling is skipped. - */ - isp->pm_domain.ops.runtime_suspend = atomisp_power_off; - isp->pm_domain.ops.runtime_resume = atomisp_power_on; - isp->pm_domain.ops.suspend = atomisp_suspend; - isp->pm_domain.ops.resume = atomisp_resume; - - cpu_latency_qos_add_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE); - dev_pm_domain_set(&pdev->dev, &isp->pm_domain); - - pm_runtime_allow(&pdev->dev); - pm_runtime_put_sync_suspend(&pdev->dev); + atomisp_pm_init(isp); err = v4l2_async_nf_register(&isp->notifier); if (err) { @@ -1471,10 +1484,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i return 0; error_unload_firmware: - pm_runtime_get_sync(&pdev->dev); - pm_runtime_forbid(&pdev->dev); - dev_pm_domain_set(&pdev->dev, NULL); - cpu_latency_qos_remove_request(&isp->pm_qos); + atomisp_pm_uninit(isp); ia_css_unload_firmware(); error_free_irq: devm_free_irq(&pdev->dev, pdev->irq, isp); @@ -1495,10 +1505,7 @@ static void atomisp_pci_remove(struct pci_dev *pdev) { struct atomisp_device *isp = pci_get_drvdata(pdev); - pm_runtime_get_sync(&pdev->dev); - pm_runtime_forbid(&pdev->dev); - dev_pm_domain_set(&pdev->dev, NULL); - cpu_latency_qos_remove_request(&isp->pm_qos); + atomisp_pm_uninit(isp); /* Undo ia_css_init() from atomisp_power_on() */ atomisp_css_uninit(isp); From patchwork Sun Dec 31 10:30:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507195 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 70EE1568B for ; Sun, 31 Dec 2023 10:31:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Tosi3jcM" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018694; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=k7BBT+mD/egNtAq4TsUSWQBDfbq6xwn3E6SHTBzXhCA=; b=Tosi3jcM/HpLrlXI4vPoSOsc7ZtM4qL7YGx/v/k/4u07SvKbISYayzzwiMUY/brnVr1zya mXJG0siHOkdKFrwTkq13z/EXdajuh9nPrAzqLoeE6SAL51wSvli+34OSbMbQYgW7Wmxw9Z CfeVtIG90wK9cIW1JnV9N8v5SR2jvYo= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-534-fGAFAzahNtitEUCYrEzxjA-1; Sun, 31 Dec 2023 05:31:30 -0500 X-MC-Unique: fGAFAzahNtitEUCYrEzxjA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0056E3811F2B; Sun, 31 Dec 2023 10:31:30 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7AA0E492BC8; Sun, 31 Dec 2023 10:31:28 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 13/15] media: atomisp: Bind and do power-management without firmware Date: Sun, 31 Dec 2023 11:30:55 +0100 Message-ID: <20231231103057.35837-14-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 The ISP needs to be turned off in a special manner for the SoC to be able to reach S0i3 during suspend. When the firmware is missing still bind to the PCI device and run in pm-only mode which takes care of turning the ISP off (including turning it off again if the firmware has turned it on during resume). In this new pm-only mode the atomisp driver works exactly the same as the non-staging, pm-only drivers/platform/x86/intel/atomisp2/pm.c driver. Signed-off-by: Hans de Goede --- .../media/atomisp/pci/atomisp_internal.h | 1 + .../staging/media/atomisp/pci/atomisp_v4l2.c | 32 ++++++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h index d5b077e602ca..bba9bc64d447 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_internal.h +++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h @@ -192,6 +192,7 @@ struct atomisp_device { struct dev_pm_domain pm_domain; struct pm_qos_request pm_qos; s32 max_isr_latency; + bool pm_only; struct atomisp_mipi_csi2_device csi2_port[ATOMISP_CAMERA_NR_PORTS]; struct atomisp_tpg_device tpg; diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index f44be2d656a8..7e241f4e9e93 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -548,7 +548,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off"); /* WA for P-Unit, if DVFS enabled, ISP timeout observed */ - if (IS_CHT && enable) { + if (IS_CHT && enable && !isp->pm_only) { punit_ddr_dvfs_enable(false); msleep(20); } @@ -558,7 +558,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) val, MRFLD_ISPSSPM0_ISPSSC_MASK); /* WA:Enable DVFS */ - if (IS_CHT && !enable) + if (IS_CHT && !enable && !isp->pm_only) punit_ddr_dvfs_enable(true); /* @@ -601,11 +601,15 @@ int atomisp_power_off(struct device *dev) int ret; u32 reg; - atomisp_css_uninit(isp); + if (isp->pm_only) { + pci_write_config_dword(pdev, PCI_INTERRUPT_CTRL, 0); + } else { + atomisp_css_uninit(isp); - ret = atomisp_mrfld_pre_power_down(isp); - if (ret) - return ret; + ret = atomisp_mrfld_pre_power_down(isp); + if (ret) + return ret; + } /* * MRFLD IUNIT DPHY is located in an always-power-on island @@ -634,6 +638,9 @@ int atomisp_power_on(struct device *dev) pci_restore_state(to_pci_dev(dev)); cpu_latency_qos_update_request(&isp->pm_qos, isp->max_isr_latency); + if (isp->pm_only) + return 0; + /*restore register values for iUnit and iUnitPHY registers*/ if (isp->saved_regs.pcicmdsts) atomisp_restore_iunit_reg(isp); @@ -1252,6 +1259,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i isp->dev = &pdev->dev; isp->saved_regs.ispmmadr = start; + isp->asd.isp = isp; mutex_init(&isp->mutex); spin_lock_init(&isp->lock); @@ -1368,8 +1376,13 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i /* Load isp firmware from user space */ isp->firmware = atomisp_load_firmware(isp); - if (!isp->firmware) - return -ENOENT; + if (!isp->firmware) { + /* No firmware continue in pm-only mode for S0i3 support */ + dev_info(&pdev->dev, "Continuing in power-management only mode\n"); + isp->pm_only = true; + atomisp_pm_init(isp); + return 0; + } err = sh_css_check_firmware_version(isp->dev, isp->firmware->data); if (err) { @@ -1507,6 +1520,9 @@ static void atomisp_pci_remove(struct pci_dev *pdev) atomisp_pm_uninit(isp); + if (isp->pm_only) + return; + /* Undo ia_css_init() from atomisp_power_on() */ atomisp_css_uninit(isp); ia_css_unload_firmware(); From patchwork Sun Dec 31 10:30:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507196 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6F4D979D4 for ; Sun, 31 Dec 2023 10:31:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="biMXuSl3" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018696; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=l1YEY+oD4lb5AgB8WSj5waUHiVWBYefH7L/B2Z7Te70=; b=biMXuSl32SEAudsmzLE6x+Mho314NKPIcvKMmDU9Zi2O6Hwc2b+lzFwVCKY9PMhXBI2jlP SKEAvbPDF43zRZrroJq33BXUM3t8mY7Y8gei1lAUbvUThOG0E37R/YmVr2AdxSzhMFi0YE v/islEbZF9tn3Pp1bxL48cPpd+Lx040= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-619-v2HdgJDwNvq18Wg55_b50Q-1; Sun, 31 Dec 2023 05:31:32 -0500 X-MC-Unique: v2HdgJDwNvq18Wg55_b50Q-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A832A85A58A; Sun, 31 Dec 2023 10:31:31 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id 31C11492BC6; Sun, 31 Dec 2023 10:31:30 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 14/15] media: atomisp: Remove unnecessary msleep(10) from atomisp_mrfld_power() error path Date: Sun, 31 Dec 2023 11:30:56 +0100 Message-ID: <20231231103057.35837-15-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Remove unnecessary msleep(10) from atomisp_mrfld_power() error-exit path, the success exit from atomisp_mrfld_power() happens if a test succeeds inside the do { } while loop above the msleep(). The error-exit path with the removed msleep is only hit it the power-on is not reflected in the iUNIT ISPSSPM0 status bits after a timeout of 50 ms. Sleeping an extra 10 ms in the timeout path makes little sense. Signed-off-by: Hans de Goede --- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 7e241f4e9e93..f736e54c7df3 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -587,9 +587,6 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) usleep_range(100, 150); } while (1); - if (enable) - msleep(10); - dev_err(isp->dev, "IUNIT power-%s timeout.\n", enable ? "on" : "off"); return -EBUSY; } From patchwork Sun Dec 31 10:30:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 13507197 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 462498486 for ; Sun, 31 Dec 2023 10:31:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="KtYAiR5/" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704018697; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FPGiWUadXxq4FZpK21b+4azGiGmvdobZBxIqZVv1gIg=; b=KtYAiR5/Ds6k24pWW3Hn2GS2pAMdbuXyWgxkVv4+2O32TClre8FHhyGmZ6uqndbt2OH5vH EivYCdL8wiZHTgKBw5s4o3qdXIN/E3lJwcPCzcsZOb/2mtlfLD3LzDypx9mbr4l+qRSxH2 GnBCY5gwFAKubYXJQLCQw5SUAUdrSoQ= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-572-Iny2euLsMz2jhF4J2DuWTw-1; Sun, 31 Dec 2023 05:31:34 -0500 X-MC-Unique: Iny2euLsMz2jhF4J2DuWTw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5BCCD832D1A; Sun, 31 Dec 2023 10:31:33 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id D9367492BC6; Sun, 31 Dec 2023 10:31:31 +0000 (UTC) From: Hans de Goede To: Mauro Carvalho Chehab , Sakari Ailus , Andy Shevchenko Cc: Hans de Goede , Kate Hsuan , Tsuchiya Yuto , Yury Luneff , Nable , andrey.i.trufanov@gmail.com, Fabio Aiuto , linux-media@vger.kernel.org, linux-staging@lists.linux.dev Subject: [PATCH 15/15] media: atomisp: Update TODO Date: Sun, 31 Dec 2023 11:30:57 +0100 Message-ID: <20231231103057.35837-16-hdegoede@redhat.com> In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com> References: <20231231103057.35837-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 Remove the TODO items for using the main (drivers/media/i2c) ov2680 and ov5693 drivers and removing the atomisp specific ones, this has been done. Remove the TODO item for gracefully handling missing firmware the "media: atomisp: Bind and do power-management without firmware" changes have fixed this. Signed-off-by: Hans de Goede --- drivers/staging/media/atomisp/TODO | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/drivers/staging/media/atomisp/TODO b/drivers/staging/media/atomisp/TODO index d99cc898cd99..bfef99997a1d 100644 --- a/drivers/staging/media/atomisp/TODO +++ b/drivers/staging/media/atomisp/TODO @@ -29,16 +29,6 @@ TODO 1. Items which MUST be fixed before the driver can be moved out of staging: -* The atomisp ov2680 and ov5693 sensor drivers bind to the same hw-ids as - the standard ov2680 and ov5693 drivers under drivers/media/i2c, which - conflicts. Drop the atomisp private ov2680 and ov5693 drivers: - * Port various ov2680 improvements from atomisp_ov2680.c to regular ov2680.c - and switch to regular ov2680 driver - * Make atomisp work with the regular ov5693 driver and drop atomisp_ov5693 - -* Fix atomisp causing the whole machine to hang in its probe() error-exit - path taken in the firmware missing case - * Remove/disable private IOCTLs * Remove/disable custom v4l2-ctrls