| Message ID | 20201126210314.7882-16-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable HDR on MCA LSPCON based Gen9 devices | expand |
Hello Uma, This expectations from user-space of having to adjust the timings of video mode doesn't seem like a good idea to me. This seems more like a quirk, and it should be better kept in I915 layer itself. Else, it will enforce user space to write a lot of vendor specific code, like: - Is it Intel device ? - Is it GEN9 ? - Is it Gen9 + LSPCON ? - Are we planning for a HDR playback ? which is not what most of the compositors developers would be interested in. I was talking to some of the Kodi folks and they also seem to think that it should go in driver. Any reason why can't we add this code in encoder->compute_config() or mode_fixup() ? compute_config() will have all the information above required, as this might be required for future LSPCON based devices as well. Regards Shashank On 27/11/20 2:33 am, Uma Shankar wrote: > Blanking needs to be reduced to incorporate DP and HDMI timing/link > bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive > 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. > This will cause mode to blank out. Reduced Htotal by shortening the > back porch and front porch within permissible limits. > > Note: This is for reference for userspace, not to be merged in kernel. > > v2: This is marked as Not for merge and the responsibilty to program > these custom timings will be on userspace. This patch is just for > reference purposes. This is based on Ville's recommendation. > > v3: updated commit message. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 0f89dbfa958a..f6f66033176b 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -741,8 +741,10 @@ intel_dp_mode_valid(struct drm_connector *connector, > { > struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector)); > struct intel_connector *intel_connector = to_intel_connector(connector); > + struct intel_encoder *intel_encoder = intel_attached_encoder(intel_connector); > struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode; > struct drm_i915_private *dev_priv = to_i915(connector->dev); > + struct intel_lspcon *lspcon = enc_to_intel_lspcon(intel_encoder); > int target_clock = mode->clock; > int max_rate, mode_rate, max_lanes, max_link_clock; > int max_dotclk = dev_priv->max_dotclk_freq; > @@ -778,6 +780,21 @@ intel_dp_mode_valid(struct drm_connector *connector, > if (target_clock > max_dotclk) > return MODE_CLOCK_HIGH; > > + /* > + * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth > + * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs > + * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will > + * cause mode to blank out. Reduced Htotal by shortening the back porch > + * and front porch within permissible limits. > + */ > + if (lspcon->active && lspcon->hdr_supported && > + mode->clock > 570000) { > + mode->clock = 570000; > + mode->htotal -= 180; > + mode->hsync_start -= 72; > + mode->hsync_end -= 72; > + } > + > max_link_clock = intel_dp_max_link_rate(intel_dp); > max_lanes = intel_dp_max_lane_count(intel_dp); >
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Shashank > Sharma > Sent: Monday, November 30, 2020 4:43 PM > To: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [v12 15/15] drm/i915/display: [NOT FOR MERGE] Reduce > blanking to support 4k60@10bpp for LSPCON > > Hello Uma, > > This expectations from user-space of having to adjust the timings of video mode > doesn't seem like a good idea to me. > > This seems more like a quirk, and it should be better kept in I915 layer itself. > > > Else, it will enforce user space to write a lot of vendor specific code, like: > > - Is it Intel device ? > > - Is it GEN9 ? > > - Is it Gen9 + LSPCON ? > > - Are we planning for a HDR playback ? > > which is not what most of the compositors developers would be interested in. > > > I was talking to some of the Kodi folks and they also seem to think that it should > go in driver. > > Any reason why can't we add this code in encoder->compute_config() or > mode_fixup() ? > > compute_config() will have all the information above required, as this might be > required for future LSPCON based devices as well. Hi Shashank, My initial idea was to have it inside kernel itself. But this actually tweaks the display timing from the standard one what sink has given, so Ville's suggestion was to keep this out from kernel, and let userspace components force the adjusted timings. This is more of a limitation coming from DP spec wrt HBR2 bandwidths. @ville.syrjala@linux.intel.com What do you suggest. I have merged the changes leaving this patch out as of now. Based on recommendation, I can re-work this and merge if we all are aligned on this one. Thanks & Regards, Uma Shankar > > Regards > > Shashank > > On 27/11/20 2:33 am, Uma Shankar wrote: > > Blanking needs to be reduced to incorporate DP and HDMI timing/link > > bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive > > 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. > > This will cause mode to blank out. Reduced Htotal by shortening the > > back porch and front porch within permissible limits. > > > > Note: This is for reference for userspace, not to be merged in kernel. > > > > v2: This is marked as Not for merge and the responsibilty to program > > these custom timings will be on userspace. This patch is just for > > reference purposes. This is based on Ville's recommendation. > > > > v3: updated commit message. > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 0f89dbfa958a..f6f66033176b 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -741,8 +741,10 @@ intel_dp_mode_valid(struct drm_connector > > *connector, { > > struct intel_dp *intel_dp = > intel_attached_dp(to_intel_connector(connector)); > > struct intel_connector *intel_connector = > > to_intel_connector(connector); > > + struct intel_encoder *intel_encoder = > > +intel_attached_encoder(intel_connector); > > struct drm_display_mode *fixed_mode = intel_connector- > >panel.fixed_mode; > > struct drm_i915_private *dev_priv = to_i915(connector->dev); > > + struct intel_lspcon *lspcon = enc_to_intel_lspcon(intel_encoder); > > int target_clock = mode->clock; > > int max_rate, mode_rate, max_lanes, max_link_clock; > > int max_dotclk = dev_priv->max_dotclk_freq; @@ -778,6 +780,21 @@ > > intel_dp_mode_valid(struct drm_connector *connector, > > if (target_clock > max_dotclk) > > return MODE_CLOCK_HIGH; > > > > + /* > > + * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth > > + * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs > > + * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will > > + * cause mode to blank out. Reduced Htotal by shortening the back > porch > > + * and front porch within permissible limits. > > + */ > > + if (lspcon->active && lspcon->hdr_supported && > > + mode->clock > 570000) { > > + mode->clock = 570000; > > + mode->htotal -= 180; > > + mode->hsync_start -= 72; > > + mode->hsync_end -= 72; > > + } > > + > > max_link_clock = intel_dp_max_link_rate(intel_dp); > > max_lanes = intel_dp_max_lane_count(intel_dp); > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 02/12/20 2:17 am, Shankar, Uma wrote: > >> -----Original Message----- >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Shashank >> Sharma >> Sent: Monday, November 30, 2020 4:43 PM >> To: intel-gfx@lists.freedesktop.org >> Subject: Re: [Intel-gfx] [v12 15/15] drm/i915/display: [NOT FOR MERGE] Reduce >> blanking to support 4k60@10bpp for LSPCON >> >> Hello Uma, >> >> This expectations from user-space of having to adjust the timings of video mode >> doesn't seem like a good idea to me. >> >> This seems more like a quirk, and it should be better kept in I915 layer itself. >> >> >> Else, it will enforce user space to write a lot of vendor specific code, like: >> >> - Is it Intel device ? >> >> - Is it GEN9 ? >> >> - Is it Gen9 + LSPCON ? >> >> - Are we planning for a HDR playback ? >> >> which is not what most of the compositors developers would be interested in. >> >> >> I was talking to some of the Kodi folks and they also seem to think that it should >> go in driver. >> >> Any reason why can't we add this code in encoder->compute_config() or >> mode_fixup() ? >> >> compute_config() will have all the information above required, as this might be >> required for future LSPCON based devices as well. > Hi Shashank, > My initial idea was to have it inside kernel itself. But this actually tweaks the display timing from the standard one > what sink has given, so Ville's suggestion was to keep this out from kernel, and let userspace components > force the adjusted timings. IIRC, we can handle this situation something like this: - Allow the mode to be in modelist - On GEN9 + LSPCON during normal modeset - Never pick 10-bit depth for 4k@60 modes (I think no changes required here) - On GEN9 + LSPCON during HDR playback (check HDR metadata) - produce the required timing and save it as "adjusted_mode" during the encoder->compute_config() Regards Shashank > > This is more of a limitation coming from DP spec wrt HBR2 bandwidths. > > @ville.syrjala@linux.intel.com What do you suggest. I have merged the changes leaving this patch out as of > now. > > Based on recommendation, I can re-work this and merge if we all are aligned on this one. > > Thanks & Regards, > Uma Shankar > >> Regards >> >> Shashank >> >> On 27/11/20 2:33 am, Uma Shankar wrote: >>> Blanking needs to be reduced to incorporate DP and HDMI timing/link >>> bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive >>> 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. >>> This will cause mode to blank out. Reduced Htotal by shortening the >>> back porch and front porch within permissible limits. >>> >>> Note: This is for reference for userspace, not to be merged in kernel. >>> >>> v2: This is marked as Not for merge and the responsibilty to program >>> these custom timings will be on userspace. This patch is just for >>> reference purposes. This is based on Ville's recommendation. >>> >>> v3: updated commit message. >>> >>> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c >>> b/drivers/gpu/drm/i915/display/intel_dp.c >>> index 0f89dbfa958a..f6f66033176b 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >>> @@ -741,8 +741,10 @@ intel_dp_mode_valid(struct drm_connector >>> *connector, { >>> struct intel_dp *intel_dp = >> intel_attached_dp(to_intel_connector(connector)); >>> struct intel_connector *intel_connector = >>> to_intel_connector(connector); >>> + struct intel_encoder *intel_encoder = >>> +intel_attached_encoder(intel_connector); >>> struct drm_display_mode *fixed_mode = intel_connector- >>> panel.fixed_mode; >>> struct drm_i915_private *dev_priv = to_i915(connector->dev); >>> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(intel_encoder); >>> int target_clock = mode->clock; >>> int max_rate, mode_rate, max_lanes, max_link_clock; >>> int max_dotclk = dev_priv->max_dotclk_freq; @@ -778,6 +780,21 @@ >>> intel_dp_mode_valid(struct drm_connector *connector, >>> if (target_clock > max_dotclk) >>> return MODE_CLOCK_HIGH; >>> >>> + /* >>> + * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth >>> + * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs >>> + * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will >>> + * cause mode to blank out. Reduced Htotal by shortening the back >> porch >>> + * and front porch within permissible limits. >>> + */ >>> + if (lspcon->active && lspcon->hdr_supported && >>> + mode->clock > 570000) { >>> + mode->clock = 570000; >>> + mode->htotal -= 180; >>> + mode->hsync_start -= 72; >>> + mode->hsync_end -= 72; >>> + } >>> + >>> max_link_clock = intel_dp_max_link_rate(intel_dp); >>> max_lanes = intel_dp_max_lane_count(intel_dp); >>> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fintel-gfx&data=04%7C01%7Cshashank.sharma%40amd.com%7C47cefaeaa67e40bfe21108d8963a5b14%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637424524669551722%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=0uxRP3Ih1HwnqJglJVF%2FL34D4KmmDZC2MOyPO7%2BE1ow%3D&reserved=0
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0f89dbfa958a..f6f66033176b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -741,8 +741,10 @@ intel_dp_mode_valid(struct drm_connector *connector, { struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector)); struct intel_connector *intel_connector = to_intel_connector(connector); + struct intel_encoder *intel_encoder = intel_attached_encoder(intel_connector); struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode; struct drm_i915_private *dev_priv = to_i915(connector->dev); + struct intel_lspcon *lspcon = enc_to_intel_lspcon(intel_encoder); int target_clock = mode->clock; int max_rate, mode_rate, max_lanes, max_link_clock; int max_dotclk = dev_priv->max_dotclk_freq; @@ -778,6 +780,21 @@ intel_dp_mode_valid(struct drm_connector *connector, if (target_clock > max_dotclk) return MODE_CLOCK_HIGH; + /* + * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth + * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs + * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will + * cause mode to blank out. Reduced Htotal by shortening the back porch + * and front porch within permissible limits. + */ + if (lspcon->active && lspcon->hdr_supported && + mode->clock > 570000) { + mode->clock = 570000; + mode->htotal -= 180; + mode->hsync_start -= 72; + mode->hsync_end -= 72; + } + max_link_clock = intel_dp_max_link_rate(intel_dp); max_lanes = intel_dp_max_lane_count(intel_dp);
Blanking needs to be reduced to incorporate DP and HDMI timing/link bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will cause mode to blank out. Reduced Htotal by shortening the back porch and front porch within permissible limits. Note: This is for reference for userspace, not to be merged in kernel. v2: This is marked as Not for merge and the responsibilty to program these custom timings will be on userspace. This patch is just for reference purposes. This is based on Ville's recommendation. v3: updated commit message. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)