From: "Niklas Söderlund" <niklas.soderlund@ragnatech.se> To: Kieran Bingham <kieran.bingham@ideasonboard.com> Cc: linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Hans Verkuil <hverkuil@xs4all.nl>, Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] media: i2c: adv748x: fix HDMI field heights Date: Mon, 8 Jan 2018 18:56:29 +0100 [thread overview] Message-ID: <20180108175629.GE23075@bigcity.dyn.berto.se> (raw) In-Reply-To: <1515433167-15912-1-git-send-email-kieran.bingham@ideasonboard.com> Hi Kieran, Thanks for your patch. On 2018-01-08 17:39:30 +0000, Kieran Bingham wrote: > The ADV748x handles interlaced media using V4L2_FIELD_ALTERNATE field > types. The correct specification for the height on the mbus is the > image height, in this instance, the field height. > > The AFE component already correctly adjusts the height on the mbus, but > the HDMI component got left behind. > > Adjust the mbus height to correctly describe the image height of the > fields when processing interlaced video for HDMI pipelines. > > Fixes: 3e89586a64df ("media: i2c: adv748x: add adv748x driver") > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/media/i2c/adv748x/adv748x-hdmi.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c > index 4da4253553fc..0e2f76f3f029 100644 > --- a/drivers/media/i2c/adv748x/adv748x-hdmi.c > +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c > @@ -105,6 +105,10 @@ static void adv748x_hdmi_fill_format(struct adv748x_hdmi *hdmi, > > fmt->width = hdmi->timings.bt.width; > fmt->height = hdmi->timings.bt.height; > + > + /* Propagate field height on the mbus for FIELD_ALTERNATE fmts */ > + if (hdmi->timings.bt.interlaced) if (V4L2_FIELD_HAS_T_OR_B(fmt->field)) Nit-picking but I would use the field here (which is set just above this in the same function) as it makes it more clear why the format is cut in half. I looked at the documentation for bt.interlaced and I'm not sure if it would be set to true for INTERLACED field formats when the height should not be halved? In this case it do not matter as fmt->field = hdmi->timings.bt.interlaced ? V4L2_FIELD_ALTERNATE : V4L2_FIELD_NONE; So I leave this up to you and feel free to add in either case. Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > + fmt->height /= 2; > } > > static void adv748x_fill_optional_dv_timings(struct v4l2_dv_timings *timings) > -- > 2.7.4 > -- Regards, Niklas Söderlund
WARNING: multiple messages have this Message-ID (diff)
From: "Niklas Söderlund" <niklas.soderlund@ragnatech.se> To: Kieran Bingham <kieran.bingham@ideasonboard.com> Cc: linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Hans Verkuil <hverkuil@xs4all.nl>, Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] media: i2c: adv748x: fix HDMI field heights Date: Mon, 8 Jan 2018 18:56:29 +0100 [thread overview] Message-ID: <20180108175629.GE23075@bigcity.dyn.berto.se> (raw) In-Reply-To: <1515433167-15912-1-git-send-email-kieran.bingham@ideasonboard.com> Hi Kieran, Thanks for your patch. On 2018-01-08 17:39:30 +0000, Kieran Bingham wrote: > The ADV748x handles interlaced media using V4L2_FIELD_ALTERNATE field > types. The correct specification for the height on the mbus is the > image height, in this instance, the field height. > > The AFE component already correctly adjusts the height on the mbus, but > the HDMI component got left behind. > > Adjust the mbus height to correctly describe the image height of the > fields when processing interlaced video for HDMI pipelines. > > Fixes: 3e89586a64df ("media: i2c: adv748x: add adv748x driver") > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/media/i2c/adv748x/adv748x-hdmi.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c b/drivers/media/i2c/adv748x/adv748x-hdmi.c > index 4da4253553fc..0e2f76f3f029 100644 > --- a/drivers/media/i2c/adv748x/adv748x-hdmi.c > +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c > @@ -105,6 +105,10 @@ static void adv748x_hdmi_fill_format(struct adv748x_hdmi *hdmi, > > fmt->width = hdmi->timings.bt.width; > fmt->height = hdmi->timings.bt.height; > + > + /* Propagate field height on the mbus for FIELD_ALTERNATE fmts */ > + if (hdmi->timings.bt.interlaced) if (V4L2_FIELD_HAS_T_OR_B(fmt->field)) Nit-picking but I would use the field here (which is set just above this in the same function) as it makes it more clear why the format is cut in half. I looked at the documentation for bt.interlaced and I'm not sure if it would be set to true for INTERLACED field formats when the height should not be halved? In this case it do not matter as fmt->field = hdmi->timings.bt.interlaced ? V4L2_FIELD_ALTERNATE : V4L2_FIELD_NONE; So I leave this up to you and feel free to add in either case. Reviewed-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se> > + fmt->height /= 2; > } > > static void adv748x_fill_optional_dv_timings(struct v4l2_dv_timings *timings) > -- > 2.7.4 > -- Regards, Niklas S�derlund
next prev parent reply other threads:[~2018-01-08 17:56 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-01-08 17:39 [PATCH] media: i2c: adv748x: fix HDMI field heights Kieran Bingham 2018-01-08 17:56 ` Niklas Söderlund [this message] 2018-01-08 17:56 ` Niklas Söderlund 2018-01-08 18:05 ` Kieran Bingham
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=20180108175629.GE23075@bigcity.dyn.berto.se \ --to=niklas.soderlund@ragnatech.se \ --cc=hverkuil@xs4all.nl \ --cc=kieran.bingham+renesas@ideasonboard.com \ --cc=kieran.bingham@ideasonboard.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=mchehab@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.