| Message ID | 20171113170427.4150-8-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13-11-2017 17:04, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > If the user mode would specify an aspect ratio other than 4:3 or 16:9 > we now silently ignore it. Maybe a better apporoach is to return an > error? Let's try that. > > Also we must be careful that we don't try to send illegal picture > aspect in the infoframe as it's only capable of signalling none, > 4:3, and 16:9. Currently we're sending these bogus infoframes > whenever the cea mode specifies some other aspect ratio. > > Yes, this is a bug. Following my input from the previous patch I think you should also check for HDMI_PICTURE_ASPECT_RESERVED or user a helper to check that. I'm also wondering if this should go to stable since the vics with aspect ratio > 16:9 were introduced. We are indeed corrupting the AVI infoframe. Best Regards, Jose Miguel Abreu
On Mon, Nov 13, 2017 at 06:30:47PM +0000, Jose Abreu wrote: > > > On 13-11-2017 17:04, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > If the user mode would specify an aspect ratio other than 4:3 or 16:9 > > we now silently ignore it. Maybe a better apporoach is to return an > > error? Let's try that. > > > > Also we must be careful that we don't try to send illegal picture > > aspect in the infoframe as it's only capable of signalling none, > > 4:3, and 16:9. Currently we're sending these bogus infoframes > > whenever the cea mode specifies some other aspect ratio. > > > > > > Yes, this is a bug. Following my input from the previous patch I > think you should also check for HDMI_PICTURE_ASPECT_RESERVED or RESERVED > 16_9 > user a helper to check that. I'm also wondering if this should go > to stable since the vics with aspect ratio > 16:9 were > introduced. We are indeed corrupting the AVI infoframe. Possibly. Just need to be careful we don't end up making a bigger mess in stable. I has some concerns about the ordering of these patches, but I guess that was between this patch and the next patch that starts to reject the illegal aspect ratios in the infoframe code. So just backporting this one alone (or both patches) should be fine.
Regards Shashank On 11/13/2017 10:34 PM, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > If the user mode would specify an aspect ratio other than 4:3 or 16:9 > we now silently ignore it. Maybe a better apporoach is to return an > error? Let's try that. > > Also we must be careful that we don't try to send illegal picture > aspect in the infoframe as it's only capable of signalling none, > 4:3, and 16:9. Currently we're sending these bogus infoframes > whenever the cea mode specifies some other aspect ratio. > > Cc: Shashank Sharma <shashank.sharma@intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Jose Abreu <Jose.Abreu@synopsys.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 00aa98f3e55d..bafb3ee4ea97 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > const struct drm_display_mode *mode, > bool is_hdmi2_sink) > { > + enum hdmi_picture_aspect picture_aspect; > int err; > > if (!frame || !mode) > @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > * Populate picture aspect ratio from either > * user input (if specified) or from the CEA mode list. > */ > - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || > - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) > - frame->picture_aspect = mode->picture_aspect_ratio; > - else if (frame->video_code > 0) > - frame->picture_aspect = drm_get_cea_aspect_ratio( > - frame->video_code); > + picture_aspect = mode->picture_aspect_ratio; > + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); This is slightly going in the loop. - During the modeset the driver cant specify the aspect ratio information, as DRM layer lacks this support. - So we fill the VIC field, by comparing the mode with the DRM_CEA_MODES[] list. This will pick the first mode available in the list (regardless of its aspect ratio), and fill the VIC, as we don't consider aspect ratio while comparing timings. - Again, now while sending the aspect ratio, we are picking up the VIC, which may not be correct. So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is first in list, we will always pick 4:3 aspect ratio. - Shashank > > + /* > + * The infoframe can't convey anything but none, 4:3 > + * and 16:9, so if the user has asked for anything else > + * we can only satisfy it by specifying the right VIC. > + */ > + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { > + if (picture_aspect != > + drm_get_cea_aspect_ratio(frame->video_code)) > + return -EINVAL; > + picture_aspect = HDMI_PICTURE_ASPECT_NONE; > + } > + > + frame->picture_aspect = picture_aspect; > frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; >
On Thu, Nov 16, 2017 at 08:31:36PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 11/13/2017 10:34 PM, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > If the user mode would specify an aspect ratio other than 4:3 or 16:9 > > we now silently ignore it. Maybe a better apporoach is to return an > > error? Let's try that. > > > > Also we must be careful that we don't try to send illegal picture > > aspect in the infoframe as it's only capable of signalling none, > > 4:3, and 16:9. Currently we're sending these bogus infoframes > > whenever the cea mode specifies some other aspect ratio. > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Jose Abreu <Jose.Abreu@synopsys.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Emil Velikov <emil.l.velikov@gmail.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------ > > 1 file changed, 17 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 00aa98f3e55d..bafb3ee4ea97 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > > const struct drm_display_mode *mode, > > bool is_hdmi2_sink) > > { > > + enum hdmi_picture_aspect picture_aspect; > > int err; > > > > if (!frame || !mode) > > @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > > * Populate picture aspect ratio from either > > * user input (if specified) or from the CEA mode list. > > */ > > - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || > > - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) > > - frame->picture_aspect = mode->picture_aspect_ratio; > > - else if (frame->video_code > 0) > > - frame->picture_aspect = drm_get_cea_aspect_ratio( > > - frame->video_code); > > + picture_aspect = mode->picture_aspect_ratio; > > + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > > + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); > This is slightly going in the loop. > - During the modeset the driver cant specify the aspect ratio > information, as DRM layer lacks this support. > - So we fill the VIC field, by comparing the mode with the > DRM_CEA_MODES[] list. This will pick the first mode > available in the list (regardless of its aspect ratio), and fill the > VIC, as we don't consider aspect ratio while comparing timings. > - Again, now while sending the aspect ratio, we are picking up the VIC, > which may not be correct. > > So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is > first in list, we will always pick 4:3 aspect ratio. Yes. The user didn't care about the aspect ratio (or rather couldn't specify one) so we just pick one. Which is exactly what we've been doing ever since we started sending the VIC in the infoframe. > > - Shashank > > > > + /* > > + * The infoframe can't convey anything but none, 4:3 > > + * and 16:9, so if the user has asked for anything else > > + * we can only satisfy it by specifying the right VIC. > > + */ > > + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { > > + if (picture_aspect != > > + drm_get_cea_aspect_ratio(frame->video_code)) > > + return -EINVAL; > > + picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > + } > > + > > + frame->picture_aspect = picture_aspect; > > frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > > frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; > >
Regards Shashank On 11/16/2017 9:56 PM, Ville Syrjälä wrote: > On Thu, Nov 16, 2017 at 08:31:36PM +0530, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 11/13/2017 10:34 PM, Ville Syrjala wrote: >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> >>> If the user mode would specify an aspect ratio other than 4:3 or 16:9 >>> we now silently ignore it. Maybe a better apporoach is to return an >>> error? Let's try that. >>> >>> Also we must be careful that we don't try to send illegal picture >>> aspect in the infoframe as it's only capable of signalling none, >>> 4:3, and 16:9. Currently we're sending these bogus infoframes >>> whenever the cea mode specifies some other aspect ratio. >>> >>> Cc: Shashank Sharma <shashank.sharma@intel.com> >>> Cc: Sean Paul <seanpaul@chromium.org> >>> Cc: Jose Abreu <Jose.Abreu@synopsys.com> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Cc: Emil Velikov <emil.l.velikov@gmail.com> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> --- >>> drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------ >>> 1 file changed, 17 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>> index 00aa98f3e55d..bafb3ee4ea97 100644 >>> --- a/drivers/gpu/drm/drm_edid.c >>> +++ b/drivers/gpu/drm/drm_edid.c >>> @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >>> const struct drm_display_mode *mode, >>> bool is_hdmi2_sink) >>> { >>> + enum hdmi_picture_aspect picture_aspect; >>> int err; >>> >>> if (!frame || !mode) >>> @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >>> * Populate picture aspect ratio from either >>> * user input (if specified) or from the CEA mode list. >>> */ >>> - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || >>> - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) >>> - frame->picture_aspect = mode->picture_aspect_ratio; >>> - else if (frame->video_code > 0) >>> - frame->picture_aspect = drm_get_cea_aspect_ratio( >>> - frame->video_code); >>> + picture_aspect = mode->picture_aspect_ratio; >>> + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) >>> + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); >> This is slightly going in the loop. >> - During the modeset the driver cant specify the aspect ratio >> information, as DRM layer lacks this support. >> - So we fill the VIC field, by comparing the mode with the >> DRM_CEA_MODES[] list. This will pick the first mode >> available in the list (regardless of its aspect ratio), and fill the >> VIC, as we don't consider aspect ratio while comparing timings. >> - Again, now while sending the aspect ratio, we are picking up the VIC, >> which may not be correct. >> >> So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is >> first in list, we will always pick 4:3 aspect ratio. > Yes. The user didn't care about the aspect ratio (or rather couldn't > specify one) so we just pick one. Which is exactly what we've been > doing ever since we started sending the VIC in the infoframe. Correct, and we are hoping that this should be better (if not fixed) with the aspect ratio support patches + DRM cap. If the userspace doesn't set the cap, then anyways there is no aspect ratio field available, and VIC would be always 0, as this becomes a Non CEA mode. Or do you think it would be a better idea to send some VIC instead of No VIC, when userspace doesn't set the DRM cap for aspect ratio ? - Shashank >> - Shashank >>> >>> + /* >>> + * The infoframe can't convey anything but none, 4:3 >>> + * and 16:9, so if the user has asked for anything else >>> + * we can only satisfy it by specifying the right VIC. >>> + */ >>> + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { >>> + if (picture_aspect != >>> + drm_get_cea_aspect_ratio(frame->video_code)) >>> + return -EINVAL; >>> + picture_aspect = HDMI_PICTURE_ASPECT_NONE; >>> + } >>> + >>> + frame->picture_aspect = picture_aspect; >>> frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; >>> frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; >>>
On Fri, Nov 17, 2017 at 08:53:54AM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 11/16/2017 9:56 PM, Ville Syrjälä wrote: > > On Thu, Nov 16, 2017 at 08:31:36PM +0530, Sharma, Shashank wrote: > >> Regards > >> > >> Shashank > >> > >> > >> On 11/13/2017 10:34 PM, Ville Syrjala wrote: > >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >>> > >>> If the user mode would specify an aspect ratio other than 4:3 or 16:9 > >>> we now silently ignore it. Maybe a better apporoach is to return an > >>> error? Let's try that. > >>> > >>> Also we must be careful that we don't try to send illegal picture > >>> aspect in the infoframe as it's only capable of signalling none, > >>> 4:3, and 16:9. Currently we're sending these bogus infoframes > >>> whenever the cea mode specifies some other aspect ratio. > >>> > >>> Cc: Shashank Sharma <shashank.sharma@intel.com> > >>> Cc: Sean Paul <seanpaul@chromium.org> > >>> Cc: Jose Abreu <Jose.Abreu@synopsys.com> > >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>> Cc: Emil Velikov <emil.l.velikov@gmail.com> > >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >>> --- > >>> drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------ > >>> 1 file changed, 17 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >>> index 00aa98f3e55d..bafb3ee4ea97 100644 > >>> --- a/drivers/gpu/drm/drm_edid.c > >>> +++ b/drivers/gpu/drm/drm_edid.c > >>> @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > >>> const struct drm_display_mode *mode, > >>> bool is_hdmi2_sink) > >>> { > >>> + enum hdmi_picture_aspect picture_aspect; > >>> int err; > >>> > >>> if (!frame || !mode) > >>> @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, > >>> * Populate picture aspect ratio from either > >>> * user input (if specified) or from the CEA mode list. > >>> */ > >>> - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || > >>> - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) > >>> - frame->picture_aspect = mode->picture_aspect_ratio; > >>> - else if (frame->video_code > 0) > >>> - frame->picture_aspect = drm_get_cea_aspect_ratio( > >>> - frame->video_code); > >>> + picture_aspect = mode->picture_aspect_ratio; > >>> + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > >>> + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); > >> This is slightly going in the loop. > >> - During the modeset the driver cant specify the aspect ratio > >> information, as DRM layer lacks this support. > >> - So we fill the VIC field, by comparing the mode with the > >> DRM_CEA_MODES[] list. This will pick the first mode > >> available in the list (regardless of its aspect ratio), and fill the > >> VIC, as we don't consider aspect ratio while comparing timings. > >> - Again, now while sending the aspect ratio, we are picking up the VIC, > >> which may not be correct. > >> > >> So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is > >> first in list, we will always pick 4:3 aspect ratio. > > Yes. The user didn't care about the aspect ratio (or rather couldn't > > specify one) so we just pick one. Which is exactly what we've been > > doing ever since we started sending the VIC in the infoframe. > Correct, and we are hoping that this should be better (if not fixed) > with the aspect ratio support > patches + DRM cap. If the userspace doesn't set the cap, then anyways > there is no aspect ratio > field available, and VIC would be always 0, as this becomes a Non CEA mode. > > Or do you think it would be a better idea to send some VIC instead of No > VIC, when userspace doesn't > set the DRM cap for aspect ratio ? Yes. That's the current behaviour. IIRC some crappy amplifiers etc. with HDMI passthrough don't even work correctly unless VIC is specified. Hence we do want to send it whenever possible. > > - Shashank > >> - Shashank > >>> > >>> + /* > >>> + * The infoframe can't convey anything but none, 4:3 > >>> + * and 16:9, so if the user has asked for anything else > >>> + * we can only satisfy it by specifying the right VIC. > >>> + */ > >>> + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { > >>> + if (picture_aspect != > >>> + drm_get_cea_aspect_ratio(frame->video_code)) > >>> + return -EINVAL; > >>> + picture_aspect = HDMI_PICTURE_ASPECT_NONE; > >>> + } > >>> + > >>> + frame->picture_aspect = picture_aspect; > >>> frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > >>> frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; > >>>
Regards Shashank On 11/17/2017 5:08 PM, Ville Syrjälä wrote: > On Fri, Nov 17, 2017 at 08:53:54AM +0530, Sharma, Shashank wrote: >> Regards >> >> Shashank >> >> >> On 11/16/2017 9:56 PM, Ville Syrjälä wrote: >>> On Thu, Nov 16, 2017 at 08:31:36PM +0530, Sharma, Shashank wrote: >>>> Regards >>>> >>>> Shashank >>>> >>>> >>>> On 11/13/2017 10:34 PM, Ville Syrjala wrote: >>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>> >>>>> If the user mode would specify an aspect ratio other than 4:3 or 16:9 >>>>> we now silently ignore it. Maybe a better apporoach is to return an >>>>> error? Let's try that. >>>>> >>>>> Also we must be careful that we don't try to send illegal picture >>>>> aspect in the infoframe as it's only capable of signalling none, >>>>> 4:3, and 16:9. Currently we're sending these bogus infoframes >>>>> whenever the cea mode specifies some other aspect ratio. >>>>> >>>>> Cc: Shashank Sharma <shashank.sharma@intel.com> >>>>> Cc: Sean Paul <seanpaul@chromium.org> >>>>> Cc: Jose Abreu <Jose.Abreu@synopsys.com> >>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>>>> Cc: Emil Velikov <emil.l.velikov@gmail.com> >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>> --- >>>>> drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------ >>>>> 1 file changed, 17 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>>>> index 00aa98f3e55d..bafb3ee4ea97 100644 >>>>> --- a/drivers/gpu/drm/drm_edid.c >>>>> +++ b/drivers/gpu/drm/drm_edid.c >>>>> @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >>>>> const struct drm_display_mode *mode, >>>>> bool is_hdmi2_sink) >>>>> { >>>>> + enum hdmi_picture_aspect picture_aspect; >>>>> int err; >>>>> >>>>> if (!frame || !mode) >>>>> @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, >>>>> * Populate picture aspect ratio from either >>>>> * user input (if specified) or from the CEA mode list. >>>>> */ >>>>> - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || >>>>> - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) >>>>> - frame->picture_aspect = mode->picture_aspect_ratio; >>>>> - else if (frame->video_code > 0) >>>>> - frame->picture_aspect = drm_get_cea_aspect_ratio( >>>>> - frame->video_code); >>>>> + picture_aspect = mode->picture_aspect_ratio; >>>>> + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) >>>>> + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); >>>> This is slightly going in the loop. >>>> - During the modeset the driver cant specify the aspect ratio >>>> information, as DRM layer lacks this support. >>>> - So we fill the VIC field, by comparing the mode with the >>>> DRM_CEA_MODES[] list. This will pick the first mode >>>> available in the list (regardless of its aspect ratio), and fill the >>>> VIC, as we don't consider aspect ratio while comparing timings. >>>> - Again, now while sending the aspect ratio, we are picking up the VIC, >>>> which may not be correct. >>>> >>>> So if we have 720x480(4:3) and 720x480(16:9) in the list, as 4:3 is >>>> first in list, we will always pick 4:3 aspect ratio. >>> Yes. The user didn't care about the aspect ratio (or rather couldn't >>> specify one) so we just pick one. Which is exactly what we've been >>> doing ever since we started sending the VIC in the infoframe. >> Correct, and we are hoping that this should be better (if not fixed) >> with the aspect ratio support >> patches + DRM cap. If the userspace doesn't set the cap, then anyways >> there is no aspect ratio >> field available, and VIC would be always 0, as this becomes a Non CEA mode. >> >> Or do you think it would be a better idea to send some VIC instead of No >> VIC, when userspace doesn't >> set the DRM cap for aspect ratio ? > Yes. That's the current behaviour. > > IIRC some crappy amplifiers etc. with HDMI passthrough don't even work > correctly unless VIC is specified. Hence we do want to send it whenever > possible. Ok, in this case this make sense, and then I kindof agree with what this patch is trying to do. Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> >> - Shashank >>>> - Shashank >>>>> >>>>> + /* >>>>> + * The infoframe can't convey anything but none, 4:3 >>>>> + * and 16:9, so if the user has asked for anything else >>>>> + * we can only satisfy it by specifying the right VIC. >>>>> + */ >>>>> + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { >>>>> + if (picture_aspect != >>>>> + drm_get_cea_aspect_ratio(frame->video_code)) >>>>> + return -EINVAL; >>>>> + picture_aspect = HDMI_PICTURE_ASPECT_NONE; >>>>> + } >>>>> + >>>>> + frame->picture_aspect = picture_aspect; >>>>> frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; >>>>> frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; >>>>>
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 00aa98f3e55d..bafb3ee4ea97 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4786,6 +4786,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, const struct drm_display_mode *mode, bool is_hdmi2_sink) { + enum hdmi_picture_aspect picture_aspect; int err; if (!frame || !mode) @@ -4828,13 +4829,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, * Populate picture aspect ratio from either * user input (if specified) or from the CEA mode list. */ - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) - frame->picture_aspect = mode->picture_aspect_ratio; - else if (frame->video_code > 0) - frame->picture_aspect = drm_get_cea_aspect_ratio( - frame->video_code); + picture_aspect = mode->picture_aspect_ratio; + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); + /* + * The infoframe can't convey anything but none, 4:3 + * and 16:9, so if the user has asked for anything else + * we can only satisfy it by specifying the right VIC. + */ + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { + if (picture_aspect != + drm_get_cea_aspect_ratio(frame->video_code)) + return -EINVAL; + picture_aspect = HDMI_PICTURE_ASPECT_NONE; + } + + frame->picture_aspect = picture_aspect; frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;