| Message ID | 1499685528-6926-9-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 10, 2017 at 04:48:36PM +0530, Shashank Sharma wrote: > A source must set output colorspace information in AVI > infoframes, so that the sink can decode upcoming frames > accordingly. > > This patch adds a function to add the output colorspace > information in the AVI infoframes. > > V2: Rebase > V3: Rebase > V4: Rebase > V5: Rebase > V6: Made patch independent of HDMI output type. > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 29 +++++++++++++++++++++++++++++ > include/drm/drm_edid.h | 5 +++++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 944a28f..cede86e 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4796,6 +4796,35 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); > > /** > + * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with > + * colorspace data of the output type > + * > + * @frame: HDMI AVI infoframe > + * @mode: DRM display mode > + * @hdmi_output: HDMI output colorspace > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +int > +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, > + const struct drm_display_mode *mode, > + enum hdmi_colorspace colorspace) > +{ > + if (colorspace > HDMI_COLORSPACE_YUV420 || > + colorspace < HDMI_COLORSPACE_RGB) { > + DRM_ERROR("Invalid color space type\n"); > + return -EINVAL; > + } Seems overly defensive. I'd say that if someone insists on writing buggy code just let them do it. > + > + frame->colorspace = colorspace; > + if (colorspace == HDMI_COLORSPACE_YUV420) > + frame->pixel_repeat = 0; Most VICs don't allow pixel repeat in 444/etc. either, and we don't protect against that. So this looks like pretty pointless check in this form. So IMO just drop this entire patch and just assign frame->colorspace in the driver. > + > + return 0; > +} > +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace); > + > +/** > * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe > * quantization range information > * @frame: HDMI AVI infoframe > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > index aa58146..b79e0cb 100644 > --- a/include/drm/drm_edid.h > +++ b/include/drm/drm_edid.h > @@ -332,6 +332,7 @@ struct cea_sad { > struct drm_encoder; > struct drm_connector; > struct drm_display_mode; > +enum drm_hdmi_output_type; > > void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid); > int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); > @@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > const struct drm_display_mode *mode, > bool is_hdmi2_sink); > int > +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, > + const struct drm_display_mode *mode, > + enum hdmi_colorspace colorspace); > +int > drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, > const struct drm_display_mode *mode); > void > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Regards Shashank On 7/12/2017 10:47 PM, Ville Syrjälä wrote: > On Mon, Jul 10, 2017 at 04:48:36PM +0530, Shashank Sharma wrote: >> A source must set output colorspace information in AVI >> infoframes, so that the sink can decode upcoming frames >> accordingly. >> >> This patch adds a function to add the output colorspace >> information in the AVI infoframes. >> >> V2: Rebase >> V3: Rebase >> V4: Rebase >> V5: Rebase >> V6: Made patch independent of HDMI output type. >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/drm_edid.c | 29 +++++++++++++++++++++++++++++ >> include/drm/drm_edid.h | 5 +++++ >> 2 files changed, 34 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 944a28f..cede86e 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -4796,6 +4796,35 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >> EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); >> >> /** >> + * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with >> + * colorspace data of the output type >> + * >> + * @frame: HDMI AVI infoframe >> + * @mode: DRM display mode >> + * @hdmi_output: HDMI output colorspace >> + * >> + * Return: 0 on success or a negative error code on failure. >> + */ >> +int >> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, >> + const struct drm_display_mode *mode, >> + enum hdmi_colorspace colorspace) >> +{ >> + if (colorspace > HDMI_COLORSPACE_YUV420 || >> + colorspace < HDMI_COLORSPACE_RGB) { >> + DRM_ERROR("Invalid color space type\n"); >> + return -EINVAL; >> + } > Seems overly defensive. I'd say that if someone insists on writing > buggy code just let them do it. :) yep can be done, you know, its a new implementation, don't want to create unnecessary noise so being a bit defensive :) >> + >> + frame->colorspace = colorspace; >> + if (colorspace == HDMI_COLORSPACE_YUV420) >> + frame->pixel_repeat = 0; > Most VICs don't allow pixel repeat in 444/etc. either, and we don't > protect against that. So this looks like pretty pointless check in > this form. > > So IMO just drop this entire patch and just assign frame->colorspace in > the driver. Actually YCBCR420 section of spec specifically calls out for not allowing repetition, also, when I tested this on a HDMI 2.0 analyzer, if was giving a AVI IF failure on pixel_repeat not 0, so IMHO it would be a good idea to keep this and get the tests passing. - Shashank >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace); >> + >> +/** >> * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe >> * quantization range information >> * @frame: HDMI AVI infoframe >> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h >> index aa58146..b79e0cb 100644 >> --- a/include/drm/drm_edid.h >> +++ b/include/drm/drm_edid.h >> @@ -332,6 +332,7 @@ struct cea_sad { >> struct drm_encoder; >> struct drm_connector; >> struct drm_display_mode; >> +enum drm_hdmi_output_type; >> >> void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid); >> int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); >> @@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >> const struct drm_display_mode *mode, >> bool is_hdmi2_sink); >> int >> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, >> + const struct drm_display_mode *mode, >> + enum hdmi_colorspace colorspace); >> +int >> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, >> const struct drm_display_mode *mode); >> void >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jul 13, 2017 at 10:37:53AM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 7/12/2017 10:47 PM, Ville Syrjälä wrote: > > On Mon, Jul 10, 2017 at 04:48:36PM +0530, Shashank Sharma wrote: > >> A source must set output colorspace information in AVI > >> infoframes, so that the sink can decode upcoming frames > >> accordingly. > >> > >> This patch adds a function to add the output colorspace > >> information in the AVI infoframes. > >> > >> V2: Rebase > >> V3: Rebase > >> V4: Rebase > >> V5: Rebase > >> V6: Made patch independent of HDMI output type. > >> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >> --- > >> drivers/gpu/drm/drm_edid.c | 29 +++++++++++++++++++++++++++++ > >> include/drm/drm_edid.h | 5 +++++ > >> 2 files changed, 34 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >> index 944a28f..cede86e 100644 > >> --- a/drivers/gpu/drm/drm_edid.c > >> +++ b/drivers/gpu/drm/drm_edid.c > >> @@ -4796,6 +4796,35 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > >> EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); > >> > >> /** > >> + * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with > >> + * colorspace data of the output type > >> + * > >> + * @frame: HDMI AVI infoframe > >> + * @mode: DRM display mode > >> + * @hdmi_output: HDMI output colorspace > >> + * > >> + * Return: 0 on success or a negative error code on failure. > >> + */ > >> +int > >> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, > >> + const struct drm_display_mode *mode, > >> + enum hdmi_colorspace colorspace) > >> +{ > >> + if (colorspace > HDMI_COLORSPACE_YUV420 || > >> + colorspace < HDMI_COLORSPACE_RGB) { > >> + DRM_ERROR("Invalid color space type\n"); > >> + return -EINVAL; > >> + } > > Seems overly defensive. I'd say that if someone insists on writing > > buggy code just let them do it. > :) yep can be done, you know, its a new implementation, don't want to > create unnecessary noise so being > a bit defensive :) > >> + > >> + frame->colorspace = colorspace; > >> + if (colorspace == HDMI_COLORSPACE_YUV420) > >> + frame->pixel_repeat = 0; > > Most VICs don't allow pixel repeat in 444/etc. either, and we don't > > protect against that. So this looks like pretty pointless check in > > this form. > > > > So IMO just drop this entire patch and just assign frame->colorspace in > > the driver. > Actually YCBCR420 section of spec specifically calls out for not > allowing repetition, also, when I tested this on a > HDMI 2.0 analyzer, if was giving a AVI IF failure on pixel_repeat not 0, > so IMHO it would be a good idea to keep > this and get the tests passing. That's just papering over bugs elsewhere. If we can't use pixel repeat with a specific mode, then we should have rejected that mode much earlier. > > - Shashank > >> + > >> + return 0; > >> +} > >> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace); > >> + > >> +/** > >> * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe > >> * quantization range information > >> * @frame: HDMI AVI infoframe > >> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > >> index aa58146..b79e0cb 100644 > >> --- a/include/drm/drm_edid.h > >> +++ b/include/drm/drm_edid.h > >> @@ -332,6 +332,7 @@ struct cea_sad { > >> struct drm_encoder; > >> struct drm_connector; > >> struct drm_display_mode; > >> +enum drm_hdmi_output_type; > >> > >> void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid); > >> int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); > >> @@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > >> const struct drm_display_mode *mode, > >> bool is_hdmi2_sink); > >> int > >> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, > >> + const struct drm_display_mode *mode, > >> + enum hdmi_colorspace colorspace); > >> +int > >> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, > >> const struct drm_display_mode *mode); > >> void > >> -- > >> 2.7.4 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Regards Shashank On 7/13/2017 6:05 PM, Ville Syrjälä wrote: > On Thu, Jul 13, 2017 at 10:37:53AM +0530, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 7/12/2017 10:47 PM, Ville Syrjälä wrote: >>> On Mon, Jul 10, 2017 at 04:48:36PM +0530, Shashank Sharma wrote: >>>> A source must set output colorspace information in AVI >>>> infoframes, so that the sink can decode upcoming frames >>>> accordingly. >>>> >>>> This patch adds a function to add the output colorspace >>>> information in the AVI infoframes. >>>> >>>> V2: Rebase >>>> V3: Rebase >>>> V4: Rebase >>>> V5: Rebase >>>> V6: Made patch independent of HDMI output type. >>>> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >>>> --- >>>> drivers/gpu/drm/drm_edid.c | 29 +++++++++++++++++++++++++++++ >>>> include/drm/drm_edid.h | 5 +++++ >>>> 2 files changed, 34 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>>> index 944a28f..cede86e 100644 >>>> --- a/drivers/gpu/drm/drm_edid.c >>>> +++ b/drivers/gpu/drm/drm_edid.c >>>> @@ -4796,6 +4796,35 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >>>> EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); >>>> >>>> /** >>>> + * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with >>>> + * colorspace data of the output type >>>> + * >>>> + * @frame: HDMI AVI infoframe >>>> + * @mode: DRM display mode >>>> + * @hdmi_output: HDMI output colorspace >>>> + * >>>> + * Return: 0 on success or a negative error code on failure. >>>> + */ >>>> +int >>>> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, >>>> + const struct drm_display_mode *mode, >>>> + enum hdmi_colorspace colorspace) >>>> +{ >>>> + if (colorspace > HDMI_COLORSPACE_YUV420 || >>>> + colorspace < HDMI_COLORSPACE_RGB) { >>>> + DRM_ERROR("Invalid color space type\n"); >>>> + return -EINVAL; >>>> + } >>> Seems overly defensive. I'd say that if someone insists on writing >>> buggy code just let them do it. >> :) yep can be done, you know, its a new implementation, don't want to >> create unnecessary noise so being >> a bit defensive :) >>>> + >>>> + frame->colorspace = colorspace; >>>> + if (colorspace == HDMI_COLORSPACE_YUV420) >>>> + frame->pixel_repeat = 0; >>> Most VICs don't allow pixel repeat in 444/etc. either, and we don't >>> protect against that. So this looks like pretty pointless check in >>> this form. >>> >>> So IMO just drop this entire patch and just assign frame->colorspace in >>> the driver. >> Actually YCBCR420 section of spec specifically calls out for not >> allowing repetition, also, when I tested this on a >> HDMI 2.0 analyzer, if was giving a AVI IF failure on pixel_repeat not 0, >> so IMHO it would be a good idea to keep >> this and get the tests passing. > That's just papering over bugs elsewhere. If we can't use pixel repeat > with a specific mode, then we should have rejected that mode much earlier. I dint get this point, HDMI 2.0 spec section 7.1 says "when YCBCR420 encoding is active, pixel repetition is not allowed" and pixel repetition field should be set to = 0, in AVI IF. This seems to be like - if you are displaying YCBCR420, set PR=0 regardless of mode, isn't it ? - Shashank >> - Shashank >>>> + >>>> + return 0; >>>> +} >>>> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace); >>>> + >>>> +/** >>>> * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe >>>> * quantization range information >>>> * @frame: HDMI AVI infoframe >>>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h >>>> index aa58146..b79e0cb 100644 >>>> --- a/include/drm/drm_edid.h >>>> +++ b/include/drm/drm_edid.h >>>> @@ -332,6 +332,7 @@ struct cea_sad { >>>> struct drm_encoder; >>>> struct drm_connector; >>>> struct drm_display_mode; >>>> +enum drm_hdmi_output_type; >>>> >>>> void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid); >>>> int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); >>>> @@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >>>> const struct drm_display_mode *mode, >>>> bool is_hdmi2_sink); >>>> int >>>> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, >>>> + const struct drm_display_mode *mode, >>>> + enum hdmi_colorspace colorspace); >>>> +int >>>> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, >>>> const struct drm_display_mode *mode); >>>> void >>>> -- >>>> 2.7.4 >>>> >>>> _______________________________________________ >>>> dri-devel mailing list >>>> dri-devel@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jul 13, 2017 at 06:19:01PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 7/13/2017 6:05 PM, Ville Syrjälä wrote: > > On Thu, Jul 13, 2017 at 10:37:53AM +0530, Sharma, Shashank wrote: > >> Regards > >> > >> Shashank > >> > >> > >> On 7/12/2017 10:47 PM, Ville Syrjälä wrote: > >>> On Mon, Jul 10, 2017 at 04:48:36PM +0530, Shashank Sharma wrote: > >>>> A source must set output colorspace information in AVI > >>>> infoframes, so that the sink can decode upcoming frames > >>>> accordingly. > >>>> > >>>> This patch adds a function to add the output colorspace > >>>> information in the AVI infoframes. > >>>> > >>>> V2: Rebase > >>>> V3: Rebase > >>>> V4: Rebase > >>>> V5: Rebase > >>>> V6: Made patch independent of HDMI output type. > >>>> > >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >>>> --- > >>>> drivers/gpu/drm/drm_edid.c | 29 +++++++++++++++++++++++++++++ > >>>> include/drm/drm_edid.h | 5 +++++ > >>>> 2 files changed, 34 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >>>> index 944a28f..cede86e 100644 > >>>> --- a/drivers/gpu/drm/drm_edid.c > >>>> +++ b/drivers/gpu/drm/drm_edid.c > >>>> @@ -4796,6 +4796,35 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > >>>> EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); > >>>> > >>>> /** > >>>> + * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with > >>>> + * colorspace data of the output type > >>>> + * > >>>> + * @frame: HDMI AVI infoframe > >>>> + * @mode: DRM display mode > >>>> + * @hdmi_output: HDMI output colorspace > >>>> + * > >>>> + * Return: 0 on success or a negative error code on failure. > >>>> + */ > >>>> +int > >>>> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, > >>>> + const struct drm_display_mode *mode, > >>>> + enum hdmi_colorspace colorspace) > >>>> +{ > >>>> + if (colorspace > HDMI_COLORSPACE_YUV420 || > >>>> + colorspace < HDMI_COLORSPACE_RGB) { > >>>> + DRM_ERROR("Invalid color space type\n"); > >>>> + return -EINVAL; > >>>> + } > >>> Seems overly defensive. I'd say that if someone insists on writing > >>> buggy code just let them do it. > >> :) yep can be done, you know, its a new implementation, don't want to > >> create unnecessary noise so being > >> a bit defensive :) > >>>> + > >>>> + frame->colorspace = colorspace; > >>>> + if (colorspace == HDMI_COLORSPACE_YUV420) > >>>> + frame->pixel_repeat = 0; > >>> Most VICs don't allow pixel repeat in 444/etc. either, and we don't > >>> protect against that. So this looks like pretty pointless check in > >>> this form. > >>> > >>> So IMO just drop this entire patch and just assign frame->colorspace in > >>> the driver. > >> Actually YCBCR420 section of spec specifically calls out for not > >> allowing repetition, also, when I tested this on a > >> HDMI 2.0 analyzer, if was giving a AVI IF failure on pixel_repeat not 0, > >> so IMHO it would be a good idea to keep > >> this and get the tests passing. > > That's just papering over bugs elsewhere. If we can't use pixel repeat > > with a specific mode, then we should have rejected that mode much earlier. > I dint get this point, HDMI 2.0 spec section 7.1 says "when YCBCR420 > encoding is active, pixel repetition is not allowed" and > pixel repetition field should be set to = 0, in AVI IF. This seems to > be like - if you are displaying YCBCR420, set PR=0 regardless of > mode, isn't it ? If the mode requires pixel repeat to meet the minimum clock requirement, then we can't just not do pixel repeat. That would violate the spec in other ways, and IIRC we couldn't even generate a low enough clock for it. > > - Shashank > >> - Shashank > >>>> + > >>>> + return 0; > >>>> +} > >>>> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace); > >>>> + > >>>> +/** > >>>> * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe > >>>> * quantization range information > >>>> * @frame: HDMI AVI infoframe > >>>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > >>>> index aa58146..b79e0cb 100644 > >>>> --- a/include/drm/drm_edid.h > >>>> +++ b/include/drm/drm_edid.h > >>>> @@ -332,6 +332,7 @@ struct cea_sad { > >>>> struct drm_encoder; > >>>> struct drm_connector; > >>>> struct drm_display_mode; > >>>> +enum drm_hdmi_output_type; > >>>> > >>>> void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid); > >>>> int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); > >>>> @@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > >>>> const struct drm_display_mode *mode, > >>>> bool is_hdmi2_sink); > >>>> int > >>>> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, > >>>> + const struct drm_display_mode *mode, > >>>> + enum hdmi_colorspace colorspace); > >>>> +int > >>>> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, > >>>> const struct drm_display_mode *mode); > >>>> void > >>>> -- > >>>> 2.7.4 > >>>> > >>>> _______________________________________________ > >>>> dri-devel mailing list > >>>> dri-devel@lists.freedesktop.org > >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 7/13/2017 6:33 PM, Ville Syrjälä wrote: > If the mode requires pixel repeat to meet the minimum clock requirement, > then we can't just not do pixel repeat. That would violate the spec in > other ways, and IIRC we couldn't even generate a low enough clock for > it. The YCBCR420 modes I am seeing on several EDIDs, are 4K modes, and they typically need a clock of 594/2 = 297Mhz, is that too low ? - Shashank
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 944a28f..cede86e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4796,6 +4796,35 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); /** + * drm_hdmi_avi_infoframe_set_colorspace - fill an HDMI AVI infoframe with + * colorspace data of the output type + * + * @frame: HDMI AVI infoframe + * @mode: DRM display mode + * @hdmi_output: HDMI output colorspace + * + * Return: 0 on success or a negative error code on failure. + */ +int +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, + const struct drm_display_mode *mode, + enum hdmi_colorspace colorspace) +{ + if (colorspace > HDMI_COLORSPACE_YUV420 || + colorspace < HDMI_COLORSPACE_RGB) { + DRM_ERROR("Invalid color space type\n"); + return -EINVAL; + } + + frame->colorspace = colorspace; + if (colorspace == HDMI_COLORSPACE_YUV420) + frame->pixel_repeat = 0; + + return 0; +} +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace); + +/** * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe * quantization range information * @frame: HDMI AVI infoframe diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index aa58146..b79e0cb 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -332,6 +332,7 @@ struct cea_sad { struct drm_encoder; struct drm_connector; struct drm_display_mode; +enum drm_hdmi_output_type; void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid); int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads); @@ -354,6 +355,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, const struct drm_display_mode *mode, bool is_hdmi2_sink); int +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame, + const struct drm_display_mode *mode, + enum hdmi_colorspace colorspace); +int drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, const struct drm_display_mode *mode); void
A source must set output colorspace information in AVI infoframes, so that the sink can decode upcoming frames accordingly. This patch adds a function to add the output colorspace information in the AVI infoframes. V2: Rebase V3: Rebase V4: Rebase V5: Rebase V6: Made patch independent of HDMI output type. Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 29 +++++++++++++++++++++++++++++ include/drm/drm_edid.h | 5 +++++ 2 files changed, 34 insertions(+)