From: Marco Felsch <m.felsch@pengutronix.de> To: Mauro Carvalho Chehab <mchehab@kernel.org> Cc: sakari.ailus@linux.intel.com, hans.verkuil@cisco.com, jacopo+renesas@jmondi.org, robh+dt@kernel.org, laurent.pinchart@ideasonboard.com, linux-media@vger.kernel.org, devicetree@vger.kernel.org, kernel@pengutronix.de Subject: Re: [PATCH v6 07/13] media: tvp5150: add FORMAT_TRY support for get/set selection handlers Date: Fri, 9 Aug 2019 07:34:01 +0200 Message-ID: <20190809053401.t2i5thhwwdbug62h@pengutronix.de> (raw) In-Reply-To: <20190514154823.1b8619b2@coco.lan> Hi Mauro, On 19-05-14 15:48, Mauro Carvalho Chehab wrote: > Em Mon, 15 Apr 2019 14:44:07 +0200 > Marco Felsch <m.felsch@pengutronix.de> escreveu: > > > Since commit 10d5509c8d50 ("[media] v4l2: remove g/s_crop from video ops") > > the 'which' field for set/get_selection must be FORMAT_ACTIVE. There is > > no way to try different selections. The patch adds a helper function to > > select the correct selection memory space (sub-device file handle or > > driver state) which will be set/returned. > > > > The TVP5150 AVID will be updated if the 'which' field is FORMAT_ACTIVE > > and the requested selection rectangle differs from the already set one. > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > Changelog: > > > > v5: > > - handle stub for v4l2_subdev_get_try_crop() internal since commit > > ("media: v4l2-subdev: add stubs for v4l2_subdev_get_try_*") > > isn't anymore part of this series. > > - add error handling of __tvp5150_get_pad_crop() > > v4: > > - fix merge conflict due to rebase on top of media-tree/master > > - __tvp5150_get_pad_crop(): cosmetic alignment fixes > > > > drivers/media/i2c/tvp5150.c | 130 ++++++++++++++++++++++++++---------- > > 1 file changed, 96 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c > > index 4e3228b2ccbc..9331609425bf 100644 > > --- a/drivers/media/i2c/tvp5150.c > > +++ b/drivers/media/i2c/tvp5150.c > > @@ -19,6 +19,7 @@ > > #include <media/v4l2-ctrls.h> > > #include <media/v4l2-fwnode.h> > > #include <media/v4l2-mc.h> > > +#include <media/v4l2-rect.h> > > > > #include "tvp5150_reg.h" > > > > @@ -997,20 +998,48 @@ static void tvp5150_set_default(v4l2_std_id std, struct v4l2_rect *crop) > > crop->height = TVP5150_V_MAX_OTHERS; > > } > > > > +static struct v4l2_rect * > > +__tvp5150_get_pad_crop(struct tvp5150 *decoder, > > + struct v4l2_subdev_pad_config *cfg, unsigned int pad, > > + enum v4l2_subdev_format_whence which) > > +{ > > + switch (which) { > > + case V4L2_SUBDEV_FORMAT_TRY: > > +#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > > + return v4l2_subdev_get_try_crop(&decoder->sd, cfg, pad); > > +#else > > + return ERR_PTR(-ENOTTY); > > +#endif > > + case V4L2_SUBDEV_FORMAT_ACTIVE: > > + return &decoder->rect; > > + default: > > + return NULL; > > + } > > Same comments as Jacopo: use return ERR_PTR(-EINVAL) instead... I applied all comments from Jacopo. Thanks for the review. Regards, Marco > > > +} > > + > > static int tvp5150_fill_fmt(struct v4l2_subdev *sd, > > struct v4l2_subdev_pad_config *cfg, > > struct v4l2_subdev_format *format) > > { > > struct v4l2_mbus_framefmt *f; > > + struct v4l2_rect *__crop; > > struct tvp5150 *decoder = to_tvp5150(sd); > > > > if (!format || (format->pad != TVP5150_PAD_VID_OUT)) > > return -EINVAL; > > > > f = &format->format; > > + __crop = __tvp5150_get_pad_crop(decoder, cfg, format->pad, > > + format->which); > > + if (IS_ERR_OR_NULL(__crop)) { > > + if (!__crop) > > + return -EINVAL; > > + else > > + return PTR_ERR(__crop); > > And here, return PTR_ERR directly. Same at the similar case below. > > > + } > > > > - f->width = decoder->rect.width; > > - f->height = decoder->rect.height / 2; > > + f->width = __crop->width; > > + f->height = __crop->height / 2; > > > > f->code = TVP5150_MBUS_FMT; > > f->field = TVP5150_FIELD; > > @@ -1021,17 +1050,51 @@ static int tvp5150_fill_fmt(struct v4l2_subdev *sd, > > return 0; > > } > > > > +unsigned int tvp5150_get_hmax(struct v4l2_subdev *sd) > > +{ > > + struct tvp5150 *decoder = to_tvp5150(sd); > > + v4l2_std_id std; > > + > > + /* Calculate height based on current standard */ > > + if (decoder->norm == V4L2_STD_ALL) > > + std = tvp5150_read_std(sd); > > + else > > + std = decoder->norm; > > + > > + return (std & V4L2_STD_525_60) ? > > + TVP5150_V_MAX_525_60 : TVP5150_V_MAX_OTHERS; > > +} > > + > > +static inline void > > +__tvp5150_set_selection(struct v4l2_subdev *sd, struct v4l2_rect rect) > > +{ > > + struct tvp5150 *decoder = to_tvp5150(sd); > > + unsigned int hmax = tvp5150_get_hmax(sd); > > + > > + regmap_write(decoder->regmap, TVP5150_VERT_BLANKING_START, rect.top); > > + regmap_write(decoder->regmap, TVP5150_VERT_BLANKING_STOP, > > + rect.top + rect.height - hmax); > > + regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_ST_MSB, > > + rect.left >> TVP5150_CROP_SHIFT); > > + regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_ST_LSB, > > + rect.left | (1 << TVP5150_CROP_SHIFT)); > > + regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_STP_MSB, > > + (rect.left + rect.width - TVP5150_MAX_CROP_LEFT) >> > > + TVP5150_CROP_SHIFT); > > + regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_STP_LSB, > > + rect.left + rect.width - TVP5150_MAX_CROP_LEFT); > > +} > > + > > static int tvp5150_set_selection(struct v4l2_subdev *sd, > > struct v4l2_subdev_pad_config *cfg, > > struct v4l2_subdev_selection *sel) > > { > > struct tvp5150 *decoder = to_tvp5150(sd); > > struct v4l2_rect rect = sel->r; > > - v4l2_std_id std; > > - int hmax; > > + struct v4l2_rect *__crop; > > + unsigned int hmax; > > > > - if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE || > > - sel->target != V4L2_SEL_TGT_CROP) > > + if (sel->target != V4L2_SEL_TGT_CROP) > > return -EINVAL; > > > > dev_dbg_lvl(sd->dev, 1, debug, "%s left=%d, top=%d, width=%d, height=%d\n", > > @@ -1040,17 +1103,7 @@ static int tvp5150_set_selection(struct v4l2_subdev *sd, > > /* tvp5150 has some special limits */ > > rect.left = clamp(rect.left, 0, TVP5150_MAX_CROP_LEFT); > > rect.top = clamp(rect.top, 0, TVP5150_MAX_CROP_TOP); > > - > > - /* Calculate height based on current standard */ > > - if (decoder->norm == V4L2_STD_ALL) > > - std = tvp5150_read_std(sd); > > - else > > - std = decoder->norm; > > - > > - if (std & V4L2_STD_525_60) > > - hmax = TVP5150_V_MAX_525_60; > > - else > > - hmax = TVP5150_V_MAX_OTHERS; > > + hmax = tvp5150_get_hmax(sd); > > > > /* > > * alignments: > > @@ -1063,20 +1116,23 @@ static int tvp5150_set_selection(struct v4l2_subdev *sd, > > hmax - TVP5150_MAX_CROP_TOP - rect.top, > > hmax - rect.top, 0, 0); > > > > - regmap_write(decoder->regmap, TVP5150_VERT_BLANKING_START, rect.top); > > - regmap_write(decoder->regmap, TVP5150_VERT_BLANKING_STOP, > > - rect.top + rect.height - hmax); > > - regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_ST_MSB, > > - rect.left >> TVP5150_CROP_SHIFT); > > - regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_ST_LSB, > > - rect.left | (1 << TVP5150_CROP_SHIFT)); > > - regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_STP_MSB, > > - (rect.left + rect.width - TVP5150_MAX_CROP_LEFT) >> > > - TVP5150_CROP_SHIFT); > > - regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_STP_LSB, > > - rect.left + rect.width - TVP5150_MAX_CROP_LEFT); > > + __crop = __tvp5150_get_pad_crop(decoder, cfg, sel->pad, sel->which); > > + if (IS_ERR_OR_NULL(__crop)) { > > + if (!__crop) > > + return -EINVAL; > > + else > > + return PTR_ERR(__crop); > > + } > > + > > + /* > > + * Update output image size if the selection (crop) rectangle size or > > + * position has been modified. > > + */ > > + if (!v4l2_rect_equal(&rect, __crop)) > > + if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE) > > + __tvp5150_set_selection(sd, rect); > > > > - decoder->rect = rect; > > + *__crop = rect; > > > > return 0; > > } > > @@ -1086,11 +1142,9 @@ static int tvp5150_get_selection(struct v4l2_subdev *sd, > > struct v4l2_subdev_selection *sel) > > { > > struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd); > > + struct v4l2_rect *__crop; > > v4l2_std_id std; > > > > - if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE) > > - return -EINVAL; > > - > > switch (sel->target) { > > case V4L2_SEL_TGT_CROP_BOUNDS: > > sel->r.left = 0; > > @@ -1108,7 +1162,15 @@ static int tvp5150_get_selection(struct v4l2_subdev *sd, > > sel->r.height = TVP5150_V_MAX_OTHERS; > > return 0; > > case V4L2_SEL_TGT_CROP: > > - sel->r = decoder->rect; > > + __crop = __tvp5150_get_pad_crop(decoder, cfg, sel->pad, > > + sel->which); > > + if (IS_ERR_OR_NULL(__crop)) { > > + if (!__crop) > > + return -EINVAL; > > + else > > + return PTR_ERR(__crop); > > + } > > + sel->r = *__crop; > > return 0; > > default: > > return -EINVAL; > > > > Thanks, > Mauro > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
next prev parent reply index Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-15 12:44 [PATCH v6 00/13] TVP5150 new features Marco Felsch 2019-04-15 12:44 ` [PATCH v6 01/13] dt-bindings: connector: analog: add tv norms property Marco Felsch 2019-05-06 10:01 ` Hans Verkuil 2019-05-06 10:06 ` Hans Verkuil 2019-05-14 18:11 ` Mauro Carvalho Chehab 2019-08-09 6:00 ` Marco Felsch 2019-05-16 16:27 ` Laurent Pinchart 2019-08-09 5:58 ` Marco Felsch 2019-08-15 12:33 ` Laurent Pinchart 2019-08-15 12:50 ` Marco Felsch 2019-08-15 13:02 ` Laurent Pinchart 2019-08-15 13:35 ` Marco Felsch 2019-04-15 12:44 ` [PATCH v6 02/13] media: v4l2-fwnode: add v4l2_fwnode_connector Marco Felsch 2019-05-06 9:50 ` Hans Verkuil 2019-05-14 18:17 ` Mauro Carvalho Chehab 2019-08-09 7:20 ` Marco Felsch 2019-05-16 16:36 ` Laurent Pinchart 2019-08-09 7:55 ` Marco Felsch 2019-08-15 12:38 ` Laurent Pinchart 2019-08-15 13:04 ` Marco Felsch 2019-08-15 13:10 ` Laurent Pinchart 2019-08-15 13:37 ` Marco Felsch 2019-04-15 12:44 ` [PATCH v6 03/13] media: v4l2-fwnode: add initial connector parsing support Marco Felsch 2019-05-06 10:10 ` Hans Verkuil 2019-05-14 18:20 ` Mauro Carvalho Chehab 2019-05-16 16:51 ` Laurent Pinchart 2019-08-09 12:16 ` Marco Felsch 2019-08-15 12:48 ` Laurent Pinchart 2019-08-15 13:14 ` Marco Felsch 2019-08-09 8:59 ` Marco Felsch 2019-04-15 12:44 ` [PATCH v6 04/13] partial revert of "[media] tvp5150: add HW input connectors support" Marco Felsch 2019-04-15 12:44 ` [PATCH v6 05/13] media: tvp5150: add input source selection of_graph support Marco Felsch 2019-05-06 10:09 ` Jacopo Mondi 2019-05-14 18:25 ` Mauro Carvalho Chehab 2019-05-16 18:03 ` Laurent Pinchart 2019-08-13 8:54 ` Marco Felsch 2019-08-15 12:51 ` Laurent Pinchart 2019-08-15 13:22 ` Marco Felsch 2019-08-15 13:26 ` Laurent Pinchart 2019-04-15 12:44 ` [PATCH v6 06/13] media: dt-bindings: tvp5150: Add input port connectors DT bindings Marco Felsch 2019-05-14 18:27 ` Mauro Carvalho Chehab 2019-05-16 18:05 ` Laurent Pinchart 2019-08-13 8:56 ` Marco Felsch 2019-04-15 12:44 ` [PATCH v6 07/13] media: tvp5150: add FORMAT_TRY support for get/set selection handlers Marco Felsch 2019-05-06 13:36 ` Jacopo Mondi 2019-08-09 5:33 ` Marco Felsch 2019-05-14 18:48 ` Mauro Carvalho Chehab 2019-08-09 5:34 ` Marco Felsch [this message] 2019-04-15 12:44 ` [PATCH v6 08/13] media: tvp5150: initialize subdev before parsing device tree Marco Felsch 2019-05-14 20:20 ` Mauro Carvalho Chehab 2019-08-09 5:42 ` Marco Felsch 2019-04-15 12:44 ` [PATCH v6 09/13] media: tvp5150: add s_power callback Marco Felsch 2019-05-14 20:13 ` Mauro Carvalho Chehab 2019-08-09 5:39 ` Marco Felsch 2019-04-15 12:44 ` [PATCH v6 10/13] media: dt-bindings: tvp5150: cleanup bindings stlye Marco Felsch 2019-04-15 12:44 ` [PATCH v6 11/13] media: dt-bindings: tvp5150: add optional tvnorms documentation Marco Felsch 2019-04-15 12:44 ` [PATCH v6 12/13] media: tvp5150: add support to limit tv norms on connector Marco Felsch 2019-05-16 18:07 ` Laurent Pinchart 2019-08-13 9:10 ` Marco Felsch 2019-08-15 12:53 ` Laurent Pinchart 2019-08-15 13:26 ` Marco Felsch 2019-04-15 12:44 ` [PATCH v6 13/13] media: tvp5150: make debug output more readable Marco Felsch 2019-05-06 13:39 ` Jacopo Mondi 2019-05-14 20:18 ` Mauro Carvalho Chehab 2019-08-09 5:42 ` Marco Felsch 2019-05-06 5:47 ` [PATCH v6 00/13] TVP5150 new features Marco Felsch 2019-05-14 17:18 ` Mauro Carvalho Chehab 2019-05-14 20:20 ` Mauro Carvalho Chehab 2019-05-14 20:58 ` Marco Felsch 2019-05-14 23:41 ` Mauro Carvalho Chehab
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190809053401.t2i5thhwwdbug62h@pengutronix.de \ --to=m.felsch@pengutronix.de \ --cc=devicetree@vger.kernel.org \ --cc=hans.verkuil@cisco.com \ --cc=jacopo+renesas@jmondi.org \ --cc=kernel@pengutronix.de \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=robh+dt@kernel.org \ --cc=sakari.ailus@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-Media Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \ linux-media@vger.kernel.org public-inbox-index linux-media Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media AGPL code for this site: git clone https://public-inbox.org/public-inbox.git