| Message ID | 1376500755-30227-7-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 14, 2013 at 06:19:09PM +0100, Damien Lespiau wrote: > Just like: > > Author: Damien Lespiau <damien.lespiau@intel.com> > Date: Mon Aug 12 11:53:24 2013 +0100 > > video/hdmi: Don't let the user of this API create invalid infoframes > > But this time for the horizontal/vertical bar data present bits. > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/video/hdmi.c | 5 +++-- > include/linux/hdmi.h | 2 -- > 2 files changed, 3 insertions(+), 4 deletions(-) Same comments as for patch 5. Although I begin to see some sense in this. Perhaps not exposing these boolean fields is a good idea after all. I wonder if we're excluding some particular use-case by not exposing these fields. Thierry
On Thu, Aug 15, 2013 at 04:45:56PM +0200, Thierry Reding wrote: > On Wed, Aug 14, 2013 at 06:19:09PM +0100, Damien Lespiau wrote: > > Just like: > > > > Author: Damien Lespiau <damien.lespiau@intel.com> > > Date: Mon Aug 12 11:53:24 2013 +0100 > > > > video/hdmi: Don't let the user of this API create invalid infoframes > > > > But this time for the horizontal/vertical bar data present bits. > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/video/hdmi.c | 5 +++-- > > include/linux/hdmi.h | 2 -- > > 2 files changed, 3 insertions(+), 4 deletions(-) > > Same comments as for patch 5. Although I begin to see some sense in > this. Perhaps not exposing these boolean fields is a good idea after > all. I wonder if we're excluding some particular use-case by not > exposing these fields. Right, so I included patch 5/6 to the v4 of the series, putting preventing someone from generating invalid infoframes before the hypothetical use-case where we want the detail of the fields. We are, de-facto, excluding the case where we'd want to have _unpack() variants decoding infoframes, but that would only be useful if we wanted to check the validity of infoframes in the kernel for instance. I think it's unlikely we'd like to do that. It's also straightforward to revert the 2 patches if/when that happens.
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index e36da36..ac84215 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -101,10 +101,11 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer, if (frame->active_aspect & 0xf) ptr[0] |= BIT(4); - if (frame->horizontal_bar_valid) + /* Bit 3 and 2 indicate if we transmit horizontal/vertical bar data */ + if (frame->top_bar || frame->bottom_bar) ptr[0] |= BIT(3); - if (frame->vertical_bar_valid) + if (frame->left_bar || frame->right_bar) ptr[0] |= BIT(2); ptr[1] = ((frame->colorimetry & 0x3) << 6) | diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 931474c6..b98340b 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -109,8 +109,6 @@ struct hdmi_avi_infoframe { unsigned char version; unsigned char length; enum hdmi_colorspace colorspace; - bool horizontal_bar_valid; - bool vertical_bar_valid; enum hdmi_scan_mode scan_mode; enum hdmi_colorimetry colorimetry; enum hdmi_picture_aspect picture_aspect;
Just like: Author: Damien Lespiau <damien.lespiau@intel.com> Date: Mon Aug 12 11:53:24 2013 +0100 video/hdmi: Don't let the user of this API create invalid infoframes But this time for the horizontal/vertical bar data present bits. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/video/hdmi.c | 5 +++-- include/linux/hdmi.h | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-)