On Thu, Nov 01, 2018 at 11:46:40PM -0700, Manasi Navare wrote: > This patch series addresses review comments on previous DSC series: > > https://patchwork.freedesktop.org/series/47514/ > > Gaurav K Singh (3): > drm/i915/dsc: Define & Compute VESA DSC params > drm/i915/dsc: Compute Rate Control parameters for DSC > drm/i915/dp: Enable/Disable DSC in DP Sink > > Manasi Navare (15): > drm/dsc: Define Display Stream Compression PPS infoframe > drm/dsc: Define VESA Display Stream Compression Capabilities > drm/dsc: Add helpers for DSC picture parameter set infoframes > drm/i915/dp: Add DSC params and DSC config to intel_crtc_state > drm/i915/dp: Compute DSC pipe config in atomic check > drm/i915/dp: Do not enable PSR2 if DSC is enabled > drm/dsc: Define the DSC 1.1 and 1.2 Line Buffer depth constants > drm/i915/dsc: Add a power domain for VDSC on eDP/MIPI DSI > drm/i915/dp: Configure i915 Picture parameter Set registers during DSC > enabling > drm/i915/dp: Use the existing write_infoframe() for DSC PPS SDPs > drm/i915/dp: Populate DSC PPS SDP and send PPS infoframes > drm/i915/dp: Configure Display stream splitter registers during DSC > enable > drm/i915/dp: Disable DSC in source by disabling DSS CTL bits > drm/i915/dsc: Enable and disable appropriate power wells for VDSC > drm/i915/dsc: Add Per connector debugfs node for DSC support/enable > > Srivatsa, Anusha (1): > drm/dsc: Define Rate Control values that do not change over > configurations I think we have these real functional issues left: - no FEC so should reject DSC on external DP - get_power_domains() thing wasn't right The potentially user triggerable DRM_ERROR()s have to be removed or explained why they can't happen (in which case a WARN() would probably be a more clear hint to the reader). The intel_dsc_enable() call I definitely would like see moved into the encoder->per_enable(). No one will think to look for it in the current location. The i915_modparams.enable_psr change seemed unrelated, but no idea if it's intentional or not. And finally there were various style nits that are optional. But I would recomment doing them since it's trivial stuff and avoids further churn in the code later. I think that's about it really. > > Documentation/gpu/drm-kms-helpers.rst | 12 + > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_dsc.c | 228 +++++ > drivers/gpu/drm/i915/Makefile | 3 +- > drivers/gpu/drm/i915/i915_debugfs.c | 71 +- > drivers/gpu/drm/i915/i915_drv.h | 4 + > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_ddi.c | 8 +- > drivers/gpu/drm/i915/intel_display.c | 28 +- > drivers/gpu/drm/i915/intel_display.h | 4 +- > drivers/gpu/drm/i915/intel_dp.c | 196 +++- > drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 21 + > drivers/gpu/drm/i915/intel_hdmi.c | 21 +- > drivers/gpu/drm/i915/intel_psr.c | 16 +- > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +- > drivers/gpu/drm/i915/intel_vdsc.c | 1100 +++++++++++++++++++++++ > include/drm/drm_dp_helper.h | 3 + > include/drm/drm_dsc.h | 485 ++++++++++ > 19 files changed, 2169 insertions(+), 40 deletions(-) > create mode 100644 drivers/gpu/drm/drm_dsc.c > create mode 100644 drivers/gpu/drm/i915/intel_vdsc.c > create mode 100644 include/drm/drm_dsc.h > > -- > 2.18.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx