| Message ID | 20191004141914.20600-4-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/edid: Make drm_get_cea_aspect_ratio() static | expand |
> -----Original Message----- > From: Ville Syrjala <ville.syrjala@linux.intel.com> > Sent: Friday, October 4, 2019 10:19 PM > To: dri-devel@lists.freedesktop.org > Cc: intel-gfx@lists.freedesktop.org; Lin, Wayne <Wayne.Lin@amd.com> > Subject: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP) > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I think this should provide most of necessary logic for adding aspecr ratios to > the HDMI 4k modes. > > Cc: Wayne Lin <waynelin@amd.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_edid.c | 37 +++++++++++++++++++++++++++++++------ > 1 file changed, 31 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index > c7f9f7ca75a2..c76814edc784 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect > drm_get_cea_aspect_ratio(const u8 video_code) > return edid_cea_modes[video_code].picture_aspect_ratio; > } > > +static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 > +video_code) { > + return edid_4k_modes[video_code].picture_aspect_ratio; > +} > + There are no picture_aspect_ratio attributes defined for modes in edid_4k_modes[] now. Should add on those definitions. > /* > * Calculate the alternate clock for HDMI modes (those from the HDMI > vendor > * specific block). > @@ -3236,6 +3241,9 @@ static u8 > drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ > if (!to_match->clock) > return 0; > > + if (to_match->picture_aspect_ratio) > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > + > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > const struct drm_display_mode *hdmi_mode = > &edid_4k_modes[vic]; > unsigned int clock1, clock2; > @@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const struct > drm_display_mode *to_match) > if (!to_match->clock) > return 0; > > + if (to_match->picture_aspect_ratio) > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > + > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > const struct drm_display_mode *hdmi_mode = > &edid_4k_modes[vic]; > unsigned int clock1, clock2; Current code in drm_match_hdmi_mdoe() & drm_match_hdmi_mode_clock_tolerance() use hdmi_mode_alternate_clock() to find alternate clocks. In hdmi_mode_alternate_clock(), it adds an exception for VIC 4 mode (4096x2160@24) due to there is no alternate clock defined for that mode in HDMI1.4b. But HDMI2.0 adds 23.98Hz for that mode. Maybe we should also revise that part. > @@ -5218,6 +5229,7 @@ > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > *frame, > const struct drm_display_mode *mode) { > enum hdmi_picture_aspect picture_aspect; > + u8 vic, hdmi_vic; > int err; > > if (!frame || !mode) > @@ -5230,7 +5242,8 @@ > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > *frame, > if (mode->flags & DRM_MODE_FLAG_DBLCLK) > frame->pixel_repeat = 1; > > - frame->video_code = drm_mode_cea_vic(connector, mode); > + vic = drm_mode_cea_vic(connector, mode); > + hdmi_vic = drm_mode_hdmi_vic(connector, mode); > > frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > @@ -5244,11 +5257,15 @@ > 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. > + * user input (if specified) or from the CEA/HDMI mode lists. > */ > picture_aspect = mode->picture_aspect_ratio; > - if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > - picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); > + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) { > + if (vic) > + picture_aspect = drm_get_cea_aspect_ratio(vic); > + else if (hdmi_vic) > + picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic); > + } > > /* > * The infoframe can't convey anything but none, 4:3 @@ -5256,12 > +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct > hdmi_avi_infoframe *frame, > * 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)) > + if (vic) { > + if (picture_aspect != drm_get_cea_aspect_ratio(vic)) > + return -EINVAL; > + } else if (hdmi_vic) { > + if (picture_aspect != drm_get_hdmi_aspect_ratio(hdmi_vic)) > + return -EINVAL; > + } else { > return -EINVAL; > + } > + > picture_aspect = HDMI_PICTURE_ASPECT_NONE; > } > > + frame->video_code = vic; > frame->picture_aspect = picture_aspect; > frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; > -- > 2.21.0 -- Wayne Lin
On Mon, Oct 14, 2019 at 09:27:07AM +0000, Lin, Wayne wrote: > > > > -----Original Message----- > > From: Ville Syrjala <ville.syrjala@linux.intel.com> > > Sent: Friday, October 4, 2019 10:19 PM > > To: dri-devel@lists.freedesktop.org > > Cc: intel-gfx@lists.freedesktop.org; Lin, Wayne <Wayne.Lin@amd.com> > > Subject: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP) > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > I think this should provide most of necessary logic for adding aspecr ratios to > > the HDMI 4k modes. > > > > Cc: Wayne Lin <waynelin@amd.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/drm_edid.c | 37 +++++++++++++++++++++++++++++++------ > > 1 file changed, 31 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index > > c7f9f7ca75a2..c76814edc784 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect > > drm_get_cea_aspect_ratio(const u8 video_code) > > return edid_cea_modes[video_code].picture_aspect_ratio; > > } > > > > +static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 > > +video_code) { > > + return edid_4k_modes[video_code].picture_aspect_ratio; > > +} > > + > > There are no picture_aspect_ratio attributes defined for modes in > edid_4k_modes[] now. Should add on those definitions. > > > /* > > * Calculate the alternate clock for HDMI modes (those from the HDMI > > vendor > > * specific block). > > @@ -3236,6 +3241,9 @@ static u8 > > drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ > > if (!to_match->clock) > > return 0; > > > > + if (to_match->picture_aspect_ratio) > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > + > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > const struct drm_display_mode *hdmi_mode = > > &edid_4k_modes[vic]; > > unsigned int clock1, clock2; > > @@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const struct > > drm_display_mode *to_match) > > if (!to_match->clock) > > return 0; > > > > + if (to_match->picture_aspect_ratio) > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > + > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > const struct drm_display_mode *hdmi_mode = > > &edid_4k_modes[vic]; > > unsigned int clock1, clock2; > > Current code in drm_match_hdmi_mdoe() & drm_match_hdmi_mode_clock_tolerance() > use hdmi_mode_alternate_clock() to find alternate clocks. > In hdmi_mode_alternate_clock(), it adds an exception for VIC 4 mode (4096x2160@24) > due to there is no alternate clock defined for that mode in HDMI1.4b. But HDMI2.0 adds > 23.98Hz for that mode. Maybe we should also revise that part. I'm tempted to just remove that exception. I have a hard time imagining it causing serious problems. > > > @@ -5218,6 +5229,7 @@ > > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > > *frame, > > const struct drm_display_mode *mode) { > > enum hdmi_picture_aspect picture_aspect; > > + u8 vic, hdmi_vic; > > int err; > > > > if (!frame || !mode) > > @@ -5230,7 +5242,8 @@ > > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > > *frame, > > if (mode->flags & DRM_MODE_FLAG_DBLCLK) > > frame->pixel_repeat = 1; > > > > - frame->video_code = drm_mode_cea_vic(connector, mode); > > + vic = drm_mode_cea_vic(connector, mode); > > + hdmi_vic = drm_mode_hdmi_vic(connector, mode); > > > > frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > > > @@ -5244,11 +5257,15 @@ > > 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. > > + * user input (if specified) or from the CEA/HDMI mode lists. > > */ > > picture_aspect = mode->picture_aspect_ratio; > > - if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > > - picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); > > + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) { > > + if (vic) > > + picture_aspect = drm_get_cea_aspect_ratio(vic); > > + else if (hdmi_vic) > > + picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic); > > + } > > > > /* > > * The infoframe can't convey anything but none, 4:3 @@ -5256,12 > > +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct > > hdmi_avi_infoframe *frame, > > * 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)) > > + if (vic) { > > + if (picture_aspect != drm_get_cea_aspect_ratio(vic)) > > + return -EINVAL; > > + } else if (hdmi_vic) { > > + if (picture_aspect != drm_get_hdmi_aspect_ratio(hdmi_vic)) > > + return -EINVAL; > > + } else { > > return -EINVAL; > > + } > > + > > picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > } > > > > + frame->video_code = vic; > > frame->picture_aspect = picture_aspect; > > frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > > frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; > > -- > > 2.21.0 > > -- > Wayne Lin
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Monday, October 14, 2019 10:42 PM > To: Lin, Wayne <Wayne.Lin@amd.com> > Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP) > > On Mon, Oct 14, 2019 at 09:27:07AM +0000, Lin, Wayne wrote: > > > > > > > -----Original Message----- > > > From: Ville Syrjala <ville.syrjala@linux.intel.com> > > > Sent: Friday, October 4, 2019 10:19 PM > > > To: dri-devel@lists.freedesktop.org > > > Cc: intel-gfx@lists.freedesktop.org; Lin, Wayne <Wayne.Lin@amd.com> > > > Subject: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP) > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > I think this should provide most of necessary logic for adding > > > aspecr ratios to the HDMI 4k modes. > > > > > > Cc: Wayne Lin <waynelin@amd.com> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/drm_edid.c | 37 > > > +++++++++++++++++++++++++++++++------ > > > 1 file changed, 31 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > > index > > > c7f9f7ca75a2..c76814edc784 100644 > > > --- a/drivers/gpu/drm/drm_edid.c > > > +++ b/drivers/gpu/drm/drm_edid.c > > > @@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect > > > drm_get_cea_aspect_ratio(const u8 video_code) > > > return edid_cea_modes[video_code].picture_aspect_ratio; > > > } > > > > > > +static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 > > > +video_code) { > > > + return edid_4k_modes[video_code].picture_aspect_ratio; > > > +} > > > + > > > > There are no picture_aspect_ratio attributes defined for modes in > > edid_4k_modes[] now. Should add on those definitions. > > > > > /* > > > * Calculate the alternate clock for HDMI modes (those from the > > > HDMI vendor > > > * specific block). > > > @@ -3236,6 +3241,9 @@ static u8 > > > drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode > *to_ > > > if (!to_match->clock) > > > return 0; > > > > > > + if (to_match->picture_aspect_ratio) > > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > > + > > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > > const struct drm_display_mode *hdmi_mode = > &edid_4k_modes[vic]; > > > unsigned int clock1, clock2; > > > @@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const struct > > > drm_display_mode *to_match) > > > if (!to_match->clock) > > > return 0; > > > > > > + if (to_match->picture_aspect_ratio) > > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > > + > > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > > const struct drm_display_mode *hdmi_mode = > &edid_4k_modes[vic]; > > > unsigned int clock1, clock2; > > > > Current code in drm_match_hdmi_mdoe() & > > drm_match_hdmi_mode_clock_tolerance() > > use hdmi_mode_alternate_clock() to find alternate clocks. > > In hdmi_mode_alternate_clock(), it adds an exception for VIC 4 mode > > (4096x2160@24) due to there is no alternate clock defined for that > > mode in HDMI1.4b. But HDMI2.0 adds 23.98Hz for that mode. Maybe we > should also revise that part. > > I'm tempted to just remove that exception. I have a hard time imagining it > causing serious problems. Thanks for your time. I've run smoke test and CTS to verify these patches (with adding the aspect ratio attribute to edid_4k_modes[] and removing the exception for23.98Hz). So far it looks good on my environment. Is there any further modification should be done on these patches? > > > > > > @@ -5218,6 +5229,7 @@ > > > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > > > *frame, > > > const struct drm_display_mode *mode) { > > > enum hdmi_picture_aspect picture_aspect; > > > + u8 vic, hdmi_vic; > > > int err; > > > > > > if (!frame || !mode) > > > @@ -5230,7 +5242,8 @@ > > > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > > > *frame, > > > if (mode->flags & DRM_MODE_FLAG_DBLCLK) > > > frame->pixel_repeat = 1; > > > > > > - frame->video_code = drm_mode_cea_vic(connector, mode); > > > + vic = drm_mode_cea_vic(connector, mode); > > > + hdmi_vic = drm_mode_hdmi_vic(connector, mode); > > > > > > frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > > > > > @@ -5244,11 +5257,15 @@ > > > 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. > > > + * user input (if specified) or from the CEA/HDMI mode lists. > > > */ > > > picture_aspect = mode->picture_aspect_ratio; > > > - if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > > > - picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); > > > + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) { > > > + if (vic) > > > + picture_aspect = drm_get_cea_aspect_ratio(vic); > > > + else if (hdmi_vic) > > > + picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic); > > > + } > > > > > > /* > > > * The infoframe can't convey anything but none, 4:3 @@ -5256,12 > > > +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct > > > hdmi_avi_infoframe *frame, > > > * 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)) > > > + if (vic) { > > > + if (picture_aspect != drm_get_cea_aspect_ratio(vic)) > > > + return -EINVAL; > > > + } else if (hdmi_vic) { > > > + if (picture_aspect != > drm_get_hdmi_aspect_ratio(hdmi_vic)) > > > + return -EINVAL; > > > + } else { > > > return -EINVAL; > > > + } > > > + > > > picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > > } > > > > > > + frame->video_code = vic; > > > frame->picture_aspect = picture_aspect; > > > frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > > > frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; > > > -- > > > 2.21.0 > > > > -- > > Wayne Lin > > -- > Ville Syrjälä > Intel -- Wayne Lin
On Mon, Oct 21, 2019 at 06:28:18AM +0000, Lin, Wayne wrote: > > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: Monday, October 14, 2019 10:42 PM > > To: Lin, Wayne <Wayne.Lin@amd.com> > > Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > Subject: Re: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP) > > > > On Mon, Oct 14, 2019 at 09:27:07AM +0000, Lin, Wayne wrote: > > > > > > > > > > -----Original Message----- > > > > From: Ville Syrjala <ville.syrjala@linux.intel.com> > > > > Sent: Friday, October 4, 2019 10:19 PM > > > > To: dri-devel@lists.freedesktop.org > > > > Cc: intel-gfx@lists.freedesktop.org; Lin, Wayne <Wayne.Lin@amd.com> > > > > Subject: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP) > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > I think this should provide most of necessary logic for adding > > > > aspecr ratios to the HDMI 4k modes. > > > > > > > > Cc: Wayne Lin <waynelin@amd.com> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > --- > > > > drivers/gpu/drm/drm_edid.c | 37 > > > > +++++++++++++++++++++++++++++++------ > > > > 1 file changed, 31 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > > > index > > > > c7f9f7ca75a2..c76814edc784 100644 > > > > --- a/drivers/gpu/drm/drm_edid.c > > > > +++ b/drivers/gpu/drm/drm_edid.c > > > > @@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect > > > > drm_get_cea_aspect_ratio(const u8 video_code) > > > > return edid_cea_modes[video_code].picture_aspect_ratio; > > > > } > > > > > > > > +static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 > > > > +video_code) { > > > > + return edid_4k_modes[video_code].picture_aspect_ratio; > > > > +} > > > > + > > > > > > There are no picture_aspect_ratio attributes defined for modes in > > > edid_4k_modes[] now. Should add on those definitions. > > > > > > > /* > > > > * Calculate the alternate clock for HDMI modes (those from the > > > > HDMI vendor > > > > * specific block). > > > > @@ -3236,6 +3241,9 @@ static u8 > > > > drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode > > *to_ > > > > if (!to_match->clock) > > > > return 0; > > > > > > > > + if (to_match->picture_aspect_ratio) > > > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > > > + > > > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > > > const struct drm_display_mode *hdmi_mode = > > &edid_4k_modes[vic]; > > > > unsigned int clock1, clock2; > > > > @@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const struct > > > > drm_display_mode *to_match) > > > > if (!to_match->clock) > > > > return 0; > > > > > > > > + if (to_match->picture_aspect_ratio) > > > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > > > + > > > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > > > const struct drm_display_mode *hdmi_mode = > > &edid_4k_modes[vic]; > > > > unsigned int clock1, clock2; > > > > > > Current code in drm_match_hdmi_mdoe() & > > > drm_match_hdmi_mode_clock_tolerance() > > > use hdmi_mode_alternate_clock() to find alternate clocks. > > > In hdmi_mode_alternate_clock(), it adds an exception for VIC 4 mode > > > (4096x2160@24) due to there is no alternate clock defined for that > > > mode in HDMI1.4b. But HDMI2.0 adds 23.98Hz for that mode. Maybe we > > should also revise that part. > > > > I'm tempted to just remove that exception. I have a hard time imagining it > > causing serious problems. > > Thanks for your time. > I've run smoke test and CTS to verify these patches (with adding the aspect ratio > attribute to edid_4k_modes[] and removing the exception for23.98Hz). So far it > looks good on my environment. Is there any further modification should be done > on these patches? Can't think of anything right now. I pushed the other three patches already, so I guess now you could pull this last patch into your patch set and repost the lot? > > > > > > > > > > @@ -5218,6 +5229,7 @@ > > > > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > > > > *frame, > > > > const struct drm_display_mode *mode) { > > > > enum hdmi_picture_aspect picture_aspect; > > > > + u8 vic, hdmi_vic; > > > > int err; > > > > > > > > if (!frame || !mode) > > > > @@ -5230,7 +5242,8 @@ > > > > drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe > > > > *frame, > > > > if (mode->flags & DRM_MODE_FLAG_DBLCLK) > > > > frame->pixel_repeat = 1; > > > > > > > > - frame->video_code = drm_mode_cea_vic(connector, mode); > > > > + vic = drm_mode_cea_vic(connector, mode); > > > > + hdmi_vic = drm_mode_hdmi_vic(connector, mode); > > > > > > > > frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > > > > > > > @@ -5244,11 +5257,15 @@ > > > > 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. > > > > + * user input (if specified) or from the CEA/HDMI mode lists. > > > > */ > > > > picture_aspect = mode->picture_aspect_ratio; > > > > - if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > > > > - picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); > > > > + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) { > > > > + if (vic) > > > > + picture_aspect = drm_get_cea_aspect_ratio(vic); > > > > + else if (hdmi_vic) > > > > + picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic); > > > > + } > > > > > > > > /* > > > > * The infoframe can't convey anything but none, 4:3 @@ -5256,12 > > > > +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct > > > > hdmi_avi_infoframe *frame, > > > > * 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)) > > > > + if (vic) { > > > > + if (picture_aspect != drm_get_cea_aspect_ratio(vic)) > > > > + return -EINVAL; > > > > + } else if (hdmi_vic) { > > > > + if (picture_aspect != > > drm_get_hdmi_aspect_ratio(hdmi_vic)) > > > > + return -EINVAL; > > > > + } else { > > > > return -EINVAL; > > > > + } > > > > + > > > > picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > > > } > > > > > > > > + frame->video_code = vic; > > > > frame->picture_aspect = picture_aspect; > > > > frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > > > > frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; > > > > -- > > > > 2.21.0 > > > > > > -- > > > Wayne Lin > > > > -- > > Ville Syrjälä > > Intel > > -- > Wayne Lin
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Saturday, October 26, 2019 3:20 AM > To: Lin, Wayne <Wayne.Lin@amd.com> > Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP) > > On Mon, Oct 21, 2019 at 06:28:18AM +0000, Lin, Wayne wrote: > > > > > > > -----Original Message----- > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Sent: Monday, October 14, 2019 10:42 PM > > > To: Lin, Wayne <Wayne.Lin@amd.com> > > > Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > > Subject: Re: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio > > > (WIP) > > > > > > On Mon, Oct 14, 2019 at 09:27:07AM +0000, Lin, Wayne wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ville Syrjala <ville.syrjala@linux.intel.com> > > > > > Sent: Friday, October 4, 2019 10:19 PM > > > > > To: dri-devel@lists.freedesktop.org > > > > > Cc: intel-gfx@lists.freedesktop.org; Lin, Wayne > > > > > <Wayne.Lin@amd.com> > > > > > Subject: [PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio > > > > > (WIP) > > > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > I think this should provide most of necessary logic for adding > > > > > aspecr ratios to the HDMI 4k modes. > > > > > > > > > > Cc: Wayne Lin <waynelin@amd.com> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > --- > > > > > drivers/gpu/drm/drm_edid.c | 37 > > > > > +++++++++++++++++++++++++++++++------ > > > > > 1 file changed, 31 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_edid.c > > > > > b/drivers/gpu/drm/drm_edid.c index > > > > > c7f9f7ca75a2..c76814edc784 100644 > > > > > --- a/drivers/gpu/drm/drm_edid.c > > > > > +++ b/drivers/gpu/drm/drm_edid.c > > > > > @@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect > > > > > drm_get_cea_aspect_ratio(const u8 video_code) > > > > > return edid_cea_modes[video_code].picture_aspect_ratio; > > > > > } > > > > > > > > > > +static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const > > > > > +u8 > > > > > +video_code) { > > > > > + return edid_4k_modes[video_code].picture_aspect_ratio; > > > > > +} > > > > > + > > > > > > > > There are no picture_aspect_ratio attributes defined for modes in > > > > edid_4k_modes[] now. Should add on those definitions. > > > > > > > > > /* > > > > > * Calculate the alternate clock for HDMI modes (those from the > > > > > HDMI vendor > > > > > * specific block). > > > > > @@ -3236,6 +3241,9 @@ static u8 > > > > > drm_match_hdmi_mode_clock_tolerance(const struct > > > > > drm_display_mode > > > *to_ > > > > > if (!to_match->clock) > > > > > return 0; > > > > > > > > > > + if (to_match->picture_aspect_ratio) > > > > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > > > > + > > > > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > > > > const struct drm_display_mode *hdmi_mode = > > > &edid_4k_modes[vic]; > > > > > unsigned int clock1, clock2; > > > > > @@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const > struct > > > > > drm_display_mode *to_match) > > > > > if (!to_match->clock) > > > > > return 0; > > > > > > > > > > + if (to_match->picture_aspect_ratio) > > > > > + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; > > > > > + > > > > > for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { > > > > > const struct drm_display_mode *hdmi_mode = > > > &edid_4k_modes[vic]; > > > > > unsigned int clock1, clock2; > > > > > > > > Current code in drm_match_hdmi_mdoe() & > > > > drm_match_hdmi_mode_clock_tolerance() > > > > use hdmi_mode_alternate_clock() to find alternate clocks. > > > > In hdmi_mode_alternate_clock(), it adds an exception for VIC 4 > > > > mode > > > > (4096x2160@24) due to there is no alternate clock defined for that > > > > mode in HDMI1.4b. But HDMI2.0 adds 23.98Hz for that mode. Maybe we > > > should also revise that part. > > > > > > I'm tempted to just remove that exception. I have a hard time > > > imagining it causing serious problems. > > > > Thanks for your time. > > I've run smoke test and CTS to verify these patches (with adding the > > aspect ratio attribute to edid_4k_modes[] and removing the exception > > for23.98Hz). So far it looks good on my environment. Is there any > > further modification should be done on these patches? > > Can't think of anything right now. I pushed the other three patches already, so I > guess now you could pull this last patch into your patch set and repost the lot? > Sorry for late reply and thanks for your time. Sure, I will post another new patch set then (including this last patch and my modification) Thanks! > > > > > > > > > > > > > > @@ -5218,6 +5229,7 @@ > > > > > drm_hdmi_avi_infoframe_from_display_mode(struct > > > > > hdmi_avi_infoframe *frame, > > > > > const struct drm_display_mode *mode) { > > > > > enum hdmi_picture_aspect picture_aspect; > > > > > + u8 vic, hdmi_vic; > > > > > int err; > > > > > > > > > > if (!frame || !mode) > > > > > @@ -5230,7 +5242,8 @@ > > > > > drm_hdmi_avi_infoframe_from_display_mode(struct > > > > > hdmi_avi_infoframe *frame, > > > > > if (mode->flags & DRM_MODE_FLAG_DBLCLK) > > > > > frame->pixel_repeat = 1; > > > > > > > > > > - frame->video_code = drm_mode_cea_vic(connector, mode); > > > > > + vic = drm_mode_cea_vic(connector, mode); > > > > > + hdmi_vic = drm_mode_hdmi_vic(connector, mode); > > > > > > > > > > frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > > > > > > > > > @@ -5244,11 +5257,15 @@ > > > > > 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. > > > > > + * user input (if specified) or from the CEA/HDMI mode lists. > > > > > */ > > > > > picture_aspect = mode->picture_aspect_ratio; > > > > > - if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) > > > > > - picture_aspect = > drm_get_cea_aspect_ratio(frame->video_code); > > > > > + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) { > > > > > + if (vic) > > > > > + picture_aspect = drm_get_cea_aspect_ratio(vic); > > > > > + else if (hdmi_vic) > > > > > + picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic); > > > > > + } > > > > > > > > > > /* > > > > > * The infoframe can't convey anything but none, 4:3 @@ > > > > > -5256,12 > > > > > +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct > > > > > hdmi_avi_infoframe *frame, > > > > > * 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)) > > > > > + if (vic) { > > > > > + if (picture_aspect != drm_get_cea_aspect_ratio(vic)) > > > > > + return -EINVAL; > > > > > + } else if (hdmi_vic) { > > > > > + if (picture_aspect != > > > drm_get_hdmi_aspect_ratio(hdmi_vic)) > > > > > + return -EINVAL; > > > > > + } else { > > > > > return -EINVAL; > > > > > + } > > > > > + > > > > > picture_aspect = HDMI_PICTURE_ASPECT_NONE; > > > > > } > > > > > > > > > > + frame->video_code = vic; > > > > > frame->picture_aspect = picture_aspect; > > > > > frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; > > > > > frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; > > > > > -- > > > > > 2.21.0 > > > > > > > > -- > > > > Wayne Lin > > > > > > -- > > > Ville Syrjälä > > > Intel > > > > -- > > Wayne Lin > > -- > Ville Syrjälä > Intel -- Wayne Lin
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c7f9f7ca75a2..c76814edc784 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code) return edid_cea_modes[video_code].picture_aspect_ratio; } +static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 video_code) +{ + return edid_4k_modes[video_code].picture_aspect_ratio; +} + /* * Calculate the alternate clock for HDMI modes (those from the HDMI vendor * specific block). @@ -3236,6 +3241,9 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ if (!to_match->clock) return 0; + if (to_match->picture_aspect_ratio) + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2; @@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if (!to_match->clock) return 0; + if (to_match->picture_aspect_ratio) + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2; @@ -5218,6 +5229,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, const struct drm_display_mode *mode) { enum hdmi_picture_aspect picture_aspect; + u8 vic, hdmi_vic; int err; if (!frame || !mode) @@ -5230,7 +5242,8 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, if (mode->flags & DRM_MODE_FLAG_DBLCLK) frame->pixel_repeat = 1; - frame->video_code = drm_mode_cea_vic(connector, mode); + vic = drm_mode_cea_vic(connector, mode); + hdmi_vic = drm_mode_hdmi_vic(connector, mode); frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; @@ -5244,11 +5257,15 @@ 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. + * user input (if specified) or from the CEA/HDMI mode lists. */ picture_aspect = mode->picture_aspect_ratio; - if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) - picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) { + if (vic) + picture_aspect = drm_get_cea_aspect_ratio(vic); + else if (hdmi_vic) + picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic); + } /* * The infoframe can't convey anything but none, 4:3 @@ -5256,12 +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, * 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)) + if (vic) { + if (picture_aspect != drm_get_cea_aspect_ratio(vic)) + return -EINVAL; + } else if (hdmi_vic) { + if (picture_aspect != drm_get_hdmi_aspect_ratio(hdmi_vic)) + return -EINVAL; + } else { return -EINVAL; + } + picture_aspect = HDMI_PICTURE_ASPECT_NONE; } + frame->video_code = vic; frame->picture_aspect = picture_aspect; frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;