From: Marco Felsch <m.felsch@pengutronix.de> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: mchehab@kernel.org, sakari.ailus@linux.intel.com, hans.verkuil@cisco.com, jacopo+renesas@jmondi.org, robh+dt@kernel.org, linux-media@vger.kernel.org, devicetree@vger.kernel.org, kernel@pengutronix.de Subject: Re: [PATCH v6 12/13] media: tvp5150: add support to limit tv norms on connector Date: Tue, 13 Aug 2019 11:10:30 +0200 Message-ID: <20190813091030.wyjjd2heoh5pk7rj@pengutronix.de> (raw) In-Reply-To: <20190516180753.GS14820@pendragon.ideasonboard.com> Hi Laurent, On 19-05-16 21:07, Laurent Pinchart wrote: > Hi Marco, > > Thank you for the patch. > > On Mon, Apr 15, 2019 at 02:44:12PM +0200, Marco Felsch wrote: > > The tvp5150 accepts NTSC(M,J,4.43), PAL (B,D,G,H,I,M,N) and SECAM video > > data and is able to auto-detect the input signal. The auto-detection > > does not work if the connector does not receive an input signal and the > > tvp5150 might not be configured correctly. This misconfiguration leads > > into wrong decoded video streams if the tvp5150 gets powered on before > > the video signal is present. > > > > Limit the supported tv norms according to the actual selected connector > > to avoid a misconfiguration. > > This seems a bit of a hack to me. In particular, on what grounds would > you specify a particular configuration in DT ? Also, this issue affects > non-DT systems, and should be solved globally. Why is this a hack? Imagine a hardware which supports PAL signals only. Then it should be forbidden for the user space to configure it to SECAM or any NTSC. Since the hardware makes the limitation it should be abstracted on DT level. Regards, Marco > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > [1] https://patchwork.kernel.org/cover/10794703/ > > > > v5: > > - probe() initialize supported tv-norms according the given connectors > > if they are available. > > - check if media-controller is used. Don't limit the norm if it isn't > > used. > > - add more logic to be smarter during connector changing so it is > > intuitiver for the user space. > > > > v2-v4: > > - nothing since the patch was squashed from series [1] into this > > series. > > > > drivers/media/i2c/tvp5150.c | 69 +++++++++++++++++++++++++++++++++++-- > > 1 file changed, 67 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c > > index cd54715eb641..c0ee08546643 100644 > > --- a/drivers/media/i2c/tvp5150.c > > +++ b/drivers/media/i2c/tvp5150.c > > @@ -32,6 +32,13 @@ > > #define TVP5150_MBUS_FMT MEDIA_BUS_FMT_UYVY8_2X8 > > #define TVP5150_FIELD V4L2_FIELD_ALTERNATE > > #define TVP5150_COLORSPACE V4L2_COLORSPACE_SMPTE170M > > +#define TVP5150_STD_MASK (V4L2_STD_NTSC | \ > > + V4L2_STD_NTSC_443 | \ > > + V4L2_STD_PAL | \ > > + V4L2_STD_PAL_M | \ > > + V4L2_STD_PAL_N | \ > > + V4L2_STD_PAL_Nc | \ > > + V4L2_STD_SECAM) > > > > MODULE_DESCRIPTION("Texas Instruments TVP5150A/TVP5150AM1/TVP5151 video decoder driver"); > > MODULE_AUTHOR("Mauro Carvalho Chehab"); > > @@ -66,6 +73,7 @@ struct tvp5150 { > > /* media-ctl properties */ > > struct media_pad pads[TVP5150_NUM_PADS]; > > struct tvp5150_connector *connectors; > > + struct tvp5150_connector *cur_connector; > > int connectors_num; > > > > struct v4l2_ctrl_handler hdl; > > @@ -785,17 +793,28 @@ static int tvp5150_g_std(struct v4l2_subdev *sd, v4l2_std_id *std) > > static int tvp5150_s_std(struct v4l2_subdev *sd, v4l2_std_id std) > > { > > struct tvp5150 *decoder = to_tvp5150(sd); > > + struct tvp5150_connector *cur_con = decoder->cur_connector; > > + v4l2_std_id supported_norms = cur_con ? > > + cur_con->base.connector.analog.supported_tvnorms : V4L2_STD_ALL; > > > > if (decoder->norm == std) > > return 0; > > > > + /* > > + * check if requested std or group of std's is/are supported by the > > + * connector > > + */ > > + if ((supported_norms & std) == 0) > > + return -EINVAL; > > + > > /* Change cropping height limits */ > > if (std & V4L2_STD_525_60) > > decoder->rect.height = TVP5150_V_MAX_525_60; > > else > > decoder->rect.height = TVP5150_V_MAX_OTHERS; > > > > - decoder->norm = std; > > + /* set only the specific supported std in case of group of std's */ > > + decoder->norm = supported_norms & std; > > > > return tvp5150_set_std(sd, std); > > } > > @@ -1347,6 +1366,8 @@ static int tvp5150_link_setup(struct media_entity *entity, > > TVP5150_BLACK_SCREEN, 0); > > > > if (flags & MEDIA_LNK_FL_ENABLED) { > > + u32 new_norm; > > + > > /* > > * S-Video connector is conneted to both ports AIP1A and AIP1B. > > * Both links must be enabled in one-shot regardless which link > > @@ -1358,6 +1379,26 @@ static int tvp5150_link_setup(struct media_entity *entity, > > if (err) > > return err; > > } > > + > > + /* Update the current connector */ > > + decoder->cur_connector = > > + container_of(remote, struct tvp5150_connector, pad); > > + > > + /* > > + * Do nothing if the new connector supports the same tv-norms as > > + * the old one. > > + */ > > + new_norm = decoder->norm & > > + decoder->cur_connector->base.connector.analog.supported_tvnorms; > > + if (decoder->norm == new_norm) > > + return 0; > > + > > + /* > > + * Fallback to the new connector tv-norms if we can't find any > > + * common between the current tv-norm and the new one. > > + */ > > + tvp5150_s_std(sd, new_norm ? new_norm : > > + decoder->cur_connector->base.connector.analog.supported_tvnorms); > > } > > > > return 0; > > @@ -1576,6 +1617,9 @@ static int tvp5150_registered(struct v4l2_subdev *sd) > > TVP5150_COMPOSITE1; > > > > tvp5150_selmux(sd); > > + decoder->cur_connector = &decoder->connectors[i]; > > + tvp5150_s_std(sd, > > + decoder->connectors[i].base.connector.analog.supported_tvnorms); > > } > > } > > #endif > > @@ -1903,6 +1947,11 @@ static int tvp5150_parse_dt(struct tvp5150 *decoder, struct device_node *np) > > ret = -EINVAL; > > goto err; > > } > > + if (!(c.connector.analog.supported_tvnorms & > > + TVP5150_STD_MASK)) > > + dev_warn(dev, > > + "Unsupported tv-norm on connector %s.\n", > > + c.label); > > in++; > > break; > > case TVP5150_PAD_VID_OUT: > > @@ -2011,7 +2060,23 @@ static int tvp5150_probe(struct i2c_client *c, > > if (res < 0) > > goto err_cleanup_dt; > > > > - core->norm = V4L2_STD_ALL; /* Default is autodetect */ > > + /* > > + * Iterate over all available connectors in case they are supported and > > + * successfully parsed. Fallback to default autodetect in case they > > + * aren't supported. > > + */ > > + if (core->connectors) { > > + struct v4l2_fwnode_connector *con; > > + int i; > > + > > + for (i = 0; i < core->connectors_num; i++) { > > + con = &core->connectors[i].base; > > + core->norm |= con->connector.analog.supported_tvnorms; > > + } > > + } else { > > + core->norm = V4L2_STD_ALL; > > + } > > + > > core->detected_norm = V4L2_STD_UNKNOWN; > > core->input = TVP5150_COMPOSITE1; > > core->enable = true; > > -- > > 2.20.1 > > > > -- > Regards, > > Laurent Pinchart > -- 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 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 [this message] 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=20190813091030.wyjjd2heoh5pk7rj@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