RE: [PATCH 04/11] drm/i915/audio: Consider fractional vdsc bpp while computing tu_data
> MTL+ supports fractional compressed bits_per_pixel, with precision of > 1/16. This compressed bpp is stored in U6.4 format. > Accommodate the precision during calculation of transfer unit data for > hblank_early calculation. > > v2: > -Fix tu_data calculation while dealing with U6.4 format. (Stan) > > v3: > -Use BPP_X16_FMT to print vdsc bpp. > LGTM. Reviewed-by: Suraj Kandpal > Signed-off-by: Ankit Nautiyal > Reviewed-by: Suraj Kandpal > Reviewed-by: Sui Jingfeng > --- > drivers/gpu/drm/i915/display/intel_audio.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c > b/drivers/gpu/drm/i915/display/intel_audio.c > index aa93ccd6c2aa..8796d90c46a6 100644 > --- a/drivers/gpu/drm/i915/display/intel_audio.c > +++ b/drivers/gpu/drm/i915/display/intel_audio.c > @@ -521,25 +521,25 @@ static unsigned int calc_hblank_early_prog(struct > intel_encoder *encoder, > unsigned int link_clks_available, link_clks_required; > unsigned int tu_data, tu_line, link_clks_active; > unsigned int h_active, h_total, hblank_delta, pixel_clk; > - unsigned int fec_coeff, cdclk, vdsc_bpp; > + unsigned int fec_coeff, cdclk, vdsc_bppx16; > unsigned int link_clk, lanes; > unsigned int hblank_rise; > > h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay; > h_total = crtc_state->hw.adjusted_mode.crtc_htotal; > pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock; > - vdsc_bpp = to_bpp_int(crtc_state->dsc.compressed_bpp_x16); > + vdsc_bppx16 = crtc_state->dsc.compressed_bpp_x16; > cdclk = i915->display.cdclk.hw.cdclk; > /* fec= 0.972261, using rounding multiplier of 100 */ > fec_coeff = 972261; > link_clk = crtc_state->port_clock; > lanes = crtc_state->lane_count; > > - drm_dbg_kms(>drm, "h_active = %u link_clk = %u :" > - "lanes = %u vdsc_bpp = %u cdclk = %u\n", > - h_active, link_clk, lanes, vdsc_bpp, cdclk); > + drm_dbg_kms(>drm, > + "h_active = %u link_clk = %u : lanes = %u vdsc_bpp = " > BPP_X16_FMT " cdclk = %u\n", > + h_active, link_clk, lanes, BPP_X16_ARGS(vdsc_bppx16), > cdclk); > > - if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bpp || !cdclk)) > + if (WARN_ON(!link_clk || !pixel_clk || !lanes || !vdsc_bppx16 || > +!cdclk)) > return 0; > > link_clks_available = (h_total - h_active) * link_clk / pixel_clk - 28; > @@ > -551,8 +551,8 @@ static unsigned int calc_hblank_early_prog(struct > intel_encoder *encoder, > hblank_delta = DIV64_U64_ROUND_UP(mul_u32_u32(5 * > (link_clk + cdclk), pixel_clk), > mul_u32_u32(link_clk, > cdclk)); > > - tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bpp * 8, > 100), > - mul_u32_u32(link_clk * lanes, fec_coeff)); > + tu_data = div64_u64(mul_u32_u32(pixel_clk * vdsc_bppx16 * 8, > 100), > + mul_u32_u32(link_clk * lanes * 16, fec_coeff)); > tu_line = div64_u64(h_active * mul_u32_u32(link_clk, fec_coeff), > mul_u32_u32(64 * pixel_clk, 100)); > link_clks_active = (tu_line - 1) * 64 + tu_data; > -- > 2.40.1
RE: [PATCH 03/11] drm/i915/display: Consider fractional vdsc bpp while computing m_n values
> MTL+ supports fractional compressed bits_per_pixel, with precision of
> 1/16. This compressed bpp is stored in U6.4 format.
> Accommodate this precision while computing m_n values.
>
> v1:
> Replace the computation of 'data_clock' with 'data_clock =
> DIV_ROUND_UP(data_clock, 16).' (Sui Jingfeng).
>
> v2:
> Rebase and pass bits_per_pixel in U6.4 format.
>
LGTM.
Reviewed-by: Suraj Kandpal
> Signed-off-by: Ankit Nautiyal
> Signed-off-by: Mitul Golani
> Reviewed-by: Suraj Kandpal
> Reviewed-by: Sui Jingfeng
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
> drivers/gpu/drm/i915/display/intel_dp.c | 16
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 +++---
> drivers/gpu/drm/i915/display/intel_fdi.c | 3 ++-
> 4 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index b4a8e3087e50..125903007a29 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2415,12 +2415,12 @@ add_bw_alloc_overhead(int link_clock, int
> bw_overhead, }
>
> void
> -intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
> +intel_link_compute_m_n(u16 bits_per_pixel_x16, int nlanes,
> int pixel_clock, int link_clock,
> int bw_overhead,
> struct intel_link_m_n *m_n)
> {
> - u32 data_clock = bits_per_pixel * pixel_clock;
> + u32 data_clock = DIV_ROUND_UP(bits_per_pixel_x16 * pixel_clock,
> 16);
> u32 data_m;
> u32 data_n;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4ad3718c3c7d..246f50d1f030 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2663,7 +2663,7 @@ static bool can_enable_drrs(struct intel_connector
> *connector, static void intel_dp_drrs_compute_config(struct intel_connector
> *connector,
>struct intel_crtc_state *pipe_config,
> - int link_bpp)
> + int link_bpp_x16)
> {
> struct drm_i915_private *i915 = to_i915(connector->base.dev);
> const struct drm_display_mode *downclock_mode = @@ -2688,7
> +2688,7 @@ intel_dp_drrs_compute_config(struct intel_connector
> *connector,
> if (pipe_config->splitter.enable)
> pixel_clock /= pipe_config->splitter.link_count;
>
> - intel_link_compute_m_n(link_bpp, pipe_config->lane_count,
> pixel_clock,
> + intel_link_compute_m_n(link_bpp_x16, pipe_config->lane_count,
> +pixel_clock,
> pipe_config->port_clock,
> intel_dp_bw_fec_overhead(pipe_config-
> >fec_enable),
> _config->dp_m2_n2);
> @@ -2792,7 +2792,7 @@ intel_dp_compute_config(struct intel_encoder
> *encoder,
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> const struct drm_display_mode *fixed_mode;
> struct intel_connector *connector = intel_dp->attached_connector;
> - int ret = 0, link_bpp;
> + int ret = 0, link_bpp_x16;
>
> if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && encoder-
> >port != PORT_A)
> pipe_config->has_pch_encoder = true;
> @@ -2841,10 +2841,10 @@ intel_dp_compute_config(struct intel_encoder
> *encoder,
> drm_dp_enhanced_frame_cap(intel_dp->dpcd);
>
> if (pipe_config->dsc.compression_enable)
> - link_bpp = to_bpp_int(pipe_config-
> >dsc.compressed_bpp_x16);
> + link_bpp_x16 = pipe_config->dsc.compressed_bpp_x16;
> else
> - link_bpp = intel_dp_output_bpp(pipe_config-
> >output_format,
> -pipe_config->pipe_bpp);
> + link_bpp_x16 =
> to_bpp_x16(intel_dp_output_bpp(pipe_config->output_format,
> + pipe_config-
> >pipe_bpp));
>
> if (intel_dp->mso_link_count) {
> int n = intel_dp->mso_link_count;
> @@ -2868,7 +2868,7 @@ intel_dp_compute_config(struct intel_encoder
> *encoder,
>
> intel_dp_audio_compute_config(encoder, pipe_config, conn_state);
>
> - intel_link_compute_m_n(link_bpp,
> + intel_link_compute_m_n(link_bpp_x16,
> pipe_config->lane_count,
> adjusted_mode->crtc_clock,
> pipe_config->port_clock,
> @@ -2884,7 +2884,7 @@ intel_dp_compute_config(struct intel_encoder
> *encoder,
>
> intel_vrr_compute_config(pipe_config, conn_state);
> intel_psr_compute_config(intel_dp, pipe_config, conn_state);
> - intel_dp_drrs_compute_config(connector, pipe_config, link_bpp);
> + intel_dp_drrs_compute_config(connector, pipe_config,
> link_bpp_x16);
> intel_dp_compute_vsc_sdp(intel_dp, pipe_config,
RE: [PATCH 02/11] drm/i915/display: Store compressed bpp in U6.4 format
> -Original Message-
> From: Nautiyal, Ankit K
> Sent: Friday, November 10, 2023 3:40 PM
> To: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org
> Cc: Sharma, Swati2 ; Kulkarni, Vandita
> ; Kandpal, Suraj ;
> suijingf...@loongson.cn
> Subject: [PATCH 02/11] drm/i915/display: Store compressed bpp in U6.4
> format
>
> DSC parameter bits_per_pixel is stored in U6.4 format.
> The 4 bits represent the fractional part of the bpp.
> Currently we use compressed_bpp member of dsc structure to store only the
> integral part of the bits_per_pixel.
> To store the full bits_per_pixel along with the fractional part,
> compressed_bpp is changed to store bpp in U6.4 formats. Intergral part is
> retrieved by simply right shifting the member compressed_bpp by 4.
>
> v2:
> -Use to_bpp_int, to_bpp_frac_dec, to_bpp_x16 helpers while dealing with
> compressed bpp. (Suraj) -Fix comment styling. (Suraj)
>
> v3:
> -Add separate file for 6.4 fixed point helper(Jani, Nikula) -Add comment for
> magic values(Suraj)
>
> v4:
> -Fix checkpatch warnings caused by renaming(Suraj)
>
> v5:
> -Rebase.
> -Use existing helpers for conversion of bpp_int to bpp_x16 and vice versa.
>
LGTM.
Reviewed-by: Suraj Kandpal
> Signed-off-by: Ankit Nautiyal
> Signed-off-by: Mitul Golani
> Reviewed-by: Suraj Kandpal
> Reviewed-by: Sui Jingfeng
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c| 10 +++
> drivers/gpu/drm/i915/display/intel_audio.c| 2 +-
> drivers/gpu/drm/i915/display/intel_bios.c | 4 +--
> drivers/gpu/drm/i915/display/intel_cdclk.c| 5 ++--
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> .../drm/i915/display/intel_display_types.h| 3 ++-
> drivers/gpu/drm/i915/display/intel_dp.c | 27 ++-
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> drivers/gpu/drm/i915/display/intel_link_bw.c | 2 +-
> drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +--
> 10 files changed, 33 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index c4585e445198..481fcb650850 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -330,7 +330,7 @@ static int afe_clk(struct intel_encoder *encoder,
> int bpp;
>
> if (crtc_state->dsc.compression_enable)
> - bpp = crtc_state->dsc.compressed_bpp;
> + bpp = to_bpp_int(crtc_state->dsc.compressed_bpp_x16);
> else
> bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi-
> >pixel_format);
>
> @@ -860,7 +860,7 @@ gen11_dsi_set_transcoder_timings(struct
> intel_encoder *encoder,
>* compressed and non-compressed bpp.
>*/
> if (crtc_state->dsc.compression_enable) {
> - mul = crtc_state->dsc.compressed_bpp;
> + mul = to_bpp_int(crtc_state->dsc.compressed_bpp_x16);
> div = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> }
>
> @@ -884,7 +884,7 @@ gen11_dsi_set_transcoder_timings(struct
> intel_encoder *encoder,
> int bpp, line_time_us, byte_clk_period_ns;
>
> if (crtc_state->dsc.compression_enable)
> - bpp = crtc_state->dsc.compressed_bpp;
> + bpp = to_bpp_int(crtc_state-
> >dsc.compressed_bpp_x16);
> else
> bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi-
> >pixel_format);
>
> @@ -1451,8 +1451,8 @@ static void gen11_dsi_get_timings(struct
> intel_encoder *encoder,
> struct drm_display_mode *adjusted_mode =
> _config->hw.adjusted_mode;
>
> - if (pipe_config->dsc.compressed_bpp) {
> - int div = pipe_config->dsc.compressed_bpp;
> + if (pipe_config->dsc.compressed_bpp_x16) {
> + int div = to_bpp_int(pipe_config->dsc.compressed_bpp_x16);
> int mul = mipi_dsi_pixel_format_to_bpp(intel_dsi-
> >pixel_format);
>
> adjusted_mode->crtc_htotal =
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index 19605264a35c..aa93ccd6c2aa 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -528,7 +528,7 @@ static unsigned int calc_hblank_early_prog(struct
> intel_encoder *encoder,
> h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock;
> -
RE: [PATCH 11/11] drm/i915/dp_mst: Add support for forcing dsc fractional bpp via debugfs
> -Original Message-
> From: Nautiyal, Ankit K
> Sent: Friday, November 10, 2023 3:40 PM
> To: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org
> Cc: Sharma, Swati2 ; Kulkarni, Vandita
> ; Kandpal, Suraj ;
> suijingf...@loongson.cn
> Subject: [PATCH 11/11] drm/i915/dp_mst: Add support for forcing dsc
> fractional bpp via debugfs
>
> If force_dsc_fractional_bpp_en is set through debugfs allow DSC iff
> compressed bpp is fractional. Continue if the computed compressed bpp
> turns out to be a integer.
>
LGTM.
Reviewed-by: Suraj Kandpal
> Signed-off-by: Ankit Nautiyal
> ---
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 322046bb7d42..26b51ba6871d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -172,6 +172,10 @@ static int
> intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
> struct intel_link_m_n remote_m_n;
> int link_bpp_x16;
>
> + if (dsc && intel_dp->force_dsc_fractional_bpp_en &&
> + !to_bpp_frac(bpp_x16))
> + continue;
> +
> drm_dbg_kms(>drm, "Trying bpp " BPP_X16_FMT "\n",
> BPP_X16_ARGS(bpp_x16));
>
> ret = intel_dp_mst_check_constraints(i915, bpp_x16,
> adjusted_mode, crtc_state, dsc); @@ -225,12 +229,16 @@ static int
> intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
> drm_dbg_kms(>drm, "failed finding vcpi slots:%d\n",
> slots);
> } else {
> - if (!dsc)
> - crtc_state->pipe_bpp = to_bpp_int(bpp_x16);
> - else
> + if (dsc) {
> crtc_state->dsc.compressed_bpp_x16 = bpp_x16;
> + if (intel_dp->force_dsc_fractional_bpp_en &&
> to_bpp_frac(bpp_x16))
> + drm_dbg_kms(>drm, "Forcing DSC
> fractional bpp\n");
> + } else {
> + crtc_state->pipe_bpp = to_bpp_int(bpp_x16);
> + }
> drm_dbg_kms(>drm, "Got %d slots for pipe bpp "
> BPP_X16_FMT " dsc %d\n",
> slots, BPP_X16_ARGS(bpp_x16), dsc);
> +
> }
>
> return slots;
> --
> 2.40.1
RE: [PATCH 10/11] drm/i916/dp_mst: Iterate over the DSC bpps as per DSC precision support
> Subject: [PATCH 10/11] drm/i916/dp_mst: Iterate over the DSC bpps as per > DSC precision support > > Currently we iterate over the bpp_x16 in step of 16. > Use DSC fractional bpp precision supported by the sink to compute the > appropriate steps to iterate over the bpps. > LGTM. Reviewed-by: Suraj Kandpal > Signed-off-by: Ankit Nautiyal > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index e7806fe11b9d..322046bb7d42 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -273,6 +273,8 @@ static int > intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder, > int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp; > u8 dsc_max_bpc; > int min_compressed_bpp, max_compressed_bpp; > + int bppx16_incr = drm_dp_dsc_sink_bpp_incr(connector- > >dp.dsc_dpcd); > + int bppx16_step; > > /* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */ > if (DISPLAY_VER(i915) >= 12) > @@ -327,11 +329,16 @@ static int > intel_dp_dsc_mst_compute_link_config(struct intel_encoder *encoder, > min_compressed_bpp = intel_dp_dsc_nearest_valid_bpp(i915, > min_compressed_bpp, > crtc_state- > >pipe_bpp); > > + if (DISPLAY_VER(i915) < 14 || bppx16_incr <= 1) > + bppx16_step = 16; > + else > + bppx16_step = 16 / bppx16_incr; > + > slots = intel_dp_mst_find_vcpi_slots_for_bpp(encoder, crtc_state, > > to_bpp_x16(max_compressed_bpp), > > to_bpp_x16(min_compressed_bpp), >limits, > - conn_state, 16, true); > + conn_state, bppx16_step, > true); > > if (slots < 0) > return slots; > -- > 2.40.1
RE: [PATCH 09/11] drm/i915/dp_mst: Use precision of 1/16 for computing bpp
> -Original Message-
> From: Nautiyal, Ankit K
> Sent: Friday, November 10, 2023 3:40 PM
> To: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org
> Cc: Sharma, Swati2 ; Kulkarni, Vandita
> ; Kandpal, Suraj ;
> suijingf...@loongson.cn
> Subject: [PATCH 09/11] drm/i915/dp_mst: Use precision of 1/16 for computing
> bpp
>
> Modify the functions to deal with bpps with 1/16 precision.
> This will make way for cases when DSC with fractional bpp is used.
> For bpp without DSC, there is no change, as we still use whole numbers.
>
LGTM.
Reviewed-by: Suraj Kandpal
> Signed-off-by: Ankit Nautiyal
> ---
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 56 +++--
> 1 file changed, 30 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 5c7e9d296483..e7806fe11b9d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -47,20 +47,21 @@
> #include "intel_vdsc.h"
> #include "skl_scaler.h"
>
> -static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int
> bpp,
> +static int intel_dp_mst_check_constraints(struct drm_i915_private
> +*i915, int bpp_x16,
> const struct drm_display_mode
> *adjusted_mode,
> struct intel_crtc_state *crtc_state,
> bool dsc)
> {
> if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) <= 13 && dsc) {
> - int output_bpp = bpp;
> + int output_bpp_x16 = bpp_x16;
> /* DisplayPort 2 128b/132b, bits per lane is always 32 */
> int symbol_clock = crtc_state->port_clock / 32;
>
> - if (output_bpp * adjusted_mode->crtc_clock >=
> + if (DIV_ROUND_UP(output_bpp_x16 * adjusted_mode-
> >crtc_clock, 16) >=
> symbol_clock * 72) {
> drm_dbg_kms(>drm, "UHBR check
> failed(required bw %d available %d)\n",
> - output_bpp * adjusted_mode->crtc_clock,
> symbol_clock * 72);
> + DIV_ROUND_UP(output_bpp_x16 *
> adjusted_mode->crtc_clock, 16),
> + symbol_clock * 72);
> return -EINVAL;
> }
> }
> @@ -127,8 +128,8 @@ static void intel_dp_mst_compute_m_n(const struct
> intel_crtc_state *crtc_state,
>
> static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder
> *encoder,
> struct intel_crtc_state
> *crtc_state,
> - int max_bpp,
> - int min_bpp,
> + int max_bpp_x16,
> + int min_bpp_x16,
> struct link_config_limits
> *limits,
> struct drm_connector_state
> *conn_state,
> int step,
> @@ -143,7 +144,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct
> intel_encoder *encoder,
> struct drm_i915_private *i915 = to_i915(connector->base.dev);
> const struct drm_display_mode *adjusted_mode =
> _state->hw.adjusted_mode;
> - int bpp, slots = -EINVAL;
> + int bpp_x16, slots = -EINVAL;
> int ret = 0;
>
> mst_state = drm_atomic_get_mst_topology_state(state, _dp-
> >mst_mgr); @@ -164,25 +165,25 @@ static int
> intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder,
> crtc_state->port_clock,
> crtc_state->lane_count);
>
> - drm_dbg_kms(>drm, "Looking for slots in range min bpp %d
> max bpp %d\n",
> - min_bpp, max_bpp);
> + drm_dbg_kms(>drm, "Looking for slots in range min bpp "
> BPP_X16_FMT " max bpp " BPP_X16_FMT "\n",
> + BPP_X16_ARGS(min_bpp_x16),
> BPP_X16_ARGS(max_bpp_x16));
>
> - for (bpp = max_bpp; bpp >= min_bpp; bpp -= step) {
> + for (bpp_x16 = max_bpp_x16; bpp_x16 >= min_bpp_x16; bpp_x16 -=
> step) {
> struct intel_link_m_n remote_m_n;
> - int link_bpp;
> + int link_bpp_x16;
>
> - drm_dbg_kms(>drm, "Trying bpp %d\n", bpp);
> +
RE: [PATCH 1/8] drm/display/dp: Add helper function to get DSC bpp prescision
> Subject: [PATCH 1/8] drm/display/dp: Add helper function to get DSC bpp
> prescision
>
> From: Ankit Nautiyal
>
> Add helper to get the DSC bits_per_pixel precision for the DP sink.
>
I think you forgot to add my reviewed by that I gave in the last revision
Anyways,
LGTM.
Reviewed-by: Suraj Kandpal
> Signed-off-by: Ankit Nautiyal
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 27 +
> include/drm/display/drm_dp_helper.h | 1 +
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 8a1b64c57dfd..5c23d5b8fc50 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2323,6 +2323,33 @@ int drm_dp_read_desc(struct drm_dp_aux *aux,
> struct drm_dp_desc *desc, } EXPORT_SYMBOL(drm_dp_read_desc);
>
> +/**
> + * drm_dp_dsc_sink_bpp_incr() - Get bits per pixel increment
> + * @dsc_dpcd: DSC capabilities from DPCD
> + *
> + * Returns the bpp precision supported by the DP sink.
> + */
> +u8 drm_dp_dsc_sink_bpp_incr(const u8
> +dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> +{
> + u8 bpp_increment_dpcd = dsc_dpcd[DP_DSC_BITS_PER_PIXEL_INC -
> +DP_DSC_SUPPORT];
> +
> + switch (bpp_increment_dpcd) {
> + case DP_DSC_BITS_PER_PIXEL_1_16:
> + return 16;
> + case DP_DSC_BITS_PER_PIXEL_1_8:
> + return 8;
> + case DP_DSC_BITS_PER_PIXEL_1_4:
> + return 4;
> + case DP_DSC_BITS_PER_PIXEL_1_2:
> + return 2;
> + case DP_DSC_BITS_PER_PIXEL_1_1:
> + return 1;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_dsc_sink_bpp_incr);
> +
> /**
> * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> * supported by the DSC sink.
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 3369104e2d25..6968d4d87931 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -164,6 +164,7 @@ drm_dp_is_branch(const u8
> dpcd[DP_RECEIVER_CAP_SIZE]) }
>
> /* DP/eDP DSC support */
> +u8 drm_dp_dsc_sink_bpp_incr(const u8
> +dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> u8 drm_dp_dsc_sink_max_slice_count(const u8
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> bool is_edp);
> u8 drm_dp_dsc_sink_line_buf_depth(const u8
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> --
> 2.25.1
RE: [PATCH 2/8] drm/i915/display: Store compressed bpp in U6.4 format
> Subject: [PATCH 2/8] drm/i915/display: Store compressed bpp in U6.4 format
>
> From: Ankit Nautiyal
>
> DSC parameter bits_per_pixel is stored in U6.4 format.
> The 4 bits represent the fractional part of the bpp.
> Currently we use compressed_bpp member of dsc structure to store only the
> integral part of the bits_per_pixel.
> To store the full bits_per_pixel along with the fractional part,
> compressed_bpp
> is changed to store bpp in U6.4 formats. Intergral part is retrieved by simply
> right shifting the member compressed_bpp by 4.
>
> v2:
> -Use to_bpp_int, to_bpp_frac_dec, to_bpp_x16 helpers while dealing with
> compressed bpp. (Suraj) -Fix comment styling. (Suraj)
>
> v3:
> -Add separate file for 6.4 fixed point helper(Jani, Nikula) -Add comment for
> magic values(Suraj)
>
A lot of checkpatch issues have been created due to the renaming you can fix
that.
Other than that everything else looks good
Regards,
Suraj Kandpal
> Signed-off-by: Ankit Nautiyal
> Signed-off-by: Mitul Golani
> Reviewed-by: Suraj Kandpal
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c| 11 +++---
> drivers/gpu/drm/i915/display/intel_audio.c| 3 +-
> drivers/gpu/drm/i915/display/intel_bios.c | 6 ++--
> drivers/gpu/drm/i915/display/intel_cdclk.c| 5 +--
> drivers/gpu/drm/i915/display/intel_display.c | 2 +-
> .../drm/i915/display/intel_display_types.h| 3 +-
> drivers/gpu/drm/i915/display/intel_dp.c | 29 ---
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 22 ++--
> .../i915/display/intel_fractional_helper.h| 36 +++
> drivers/gpu/drm/i915/display/intel_vdsc.c | 5 +--
> 10 files changed, 85 insertions(+), 37 deletions(-) create mode 100644
> drivers/gpu/drm/i915/display/intel_fractional_helper.h
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index ad6488e9c2b2..0f7594b6aa1f 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -43,6 +43,7 @@
> #include "intel_de.h"
> #include "intel_dsi.h"
> #include "intel_dsi_vbt.h"
> +#include "intel_fractional_helper.h"
> #include "intel_panel.h"
> #include "intel_vdsc.h"
> #include "intel_vdsc_regs.h"
> @@ -330,7 +331,7 @@ static int afe_clk(struct intel_encoder *encoder,
> int bpp;
>
> if (crtc_state->dsc.compression_enable)
> - bpp = crtc_state->dsc.compressed_bpp;
> + bpp =
> +intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16);
> else
> bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
>
> @@ -860,7 +861,7 @@ gen11_dsi_set_transcoder_timings(struct
> intel_encoder *encoder,
>* compressed and non-compressed bpp.
>*/
> if (crtc_state->dsc.compression_enable) {
> - mul = crtc_state->dsc.compressed_bpp;
> + mul =
> +intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16);
> div = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> }
>
> @@ -884,7 +885,7 @@ gen11_dsi_set_transcoder_timings(struct
> intel_encoder *encoder,
> int bpp, line_time_us, byte_clk_period_ns;
>
> if (crtc_state->dsc.compression_enable)
> - bpp = crtc_state->dsc.compressed_bpp;
> + bpp =
> +intel_fractional_bpp_from_x16(crtc_state->dsc.compressed_bpp_x16);
> else
> bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi-
> >pixel_format);
>
> @@ -1451,8 +1452,8 @@ static void gen11_dsi_get_timings(struct
> intel_encoder *encoder,
> struct drm_display_mode *adjusted_mode =
> _config->hw.adjusted_mode;
>
> - if (pipe_config->dsc.compressed_bpp) {
> - int div = pipe_config->dsc.compressed_bpp;
> + if (pipe_config->dsc.compressed_bpp_x16) {
> + int div =
> +intel_fractional_bpp_from_x16(pipe_config->dsc.compressed_bpp_x16);
> int mul = mipi_dsi_pixel_format_to_bpp(intel_dsi-
> >pixel_format);
>
> adjusted_mode->crtc_htotal =
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> b/drivers/gpu/drm/i915/display/intel_audio.c
> index 19605264a35c..4f1db1581316 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -35,6 +35,7 @@
> #include "intel_crtc.h"
> #include "intel_de.h"
> #include "intel_display_types.h"
> +#include "intel_fractional_helper.h"
> #include "intel_lpe_audio.h"
>
> /**
> @@ -528,7 +529,7 @@ static unsigned int calc_hblank_early_prog(struct
> intel_encoder *encoder,
> h_active = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> h_total = crtc_state->hw.adjusted_mode.crtc_htotal;
> pixel_clk = crtc_state->hw.adjusted_mode.crtc_clock;
> - vdsc_bpp = crtc_state->dsc.compressed_bpp;
> + vdsc_bpp =
>
RE: [PATCH 1/8] drm/display/dp: Add helper function to get DSC bpp prescision
> Subject: [PATCH 1/8] drm/display/dp: Add helper function to get DSC bpp
> prescision
>
> From: Ankit Nautiyal
>
> Add helper to get the DSC bits_per_pixel precision for the DP sink.
>
LGTM.
Reviewed-by: Suraj Kandpal
> Signed-off-by: Ankit Nautiyal
> ---
> drivers/gpu/drm/display/drm_dp_helper.c | 27 +
> include/drm/display/drm_dp_helper.h | 1 +
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> b/drivers/gpu/drm/display/drm_dp_helper.c
> index 8a1b64c57dfd..5c23d5b8fc50 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2323,6 +2323,33 @@ int drm_dp_read_desc(struct drm_dp_aux *aux,
> struct drm_dp_desc *desc, } EXPORT_SYMBOL(drm_dp_read_desc);
>
> +/**
> + * drm_dp_dsc_sink_bpp_incr() - Get bits per pixel increment
> + * @dsc_dpcd: DSC capabilities from DPCD
> + *
> + * Returns the bpp precision supported by the DP sink.
> + */
> +u8 drm_dp_dsc_sink_bpp_incr(const u8
> +dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
> +{
> + u8 bpp_increment_dpcd = dsc_dpcd[DP_DSC_BITS_PER_PIXEL_INC -
> +DP_DSC_SUPPORT];
> +
> + switch (bpp_increment_dpcd) {
> + case DP_DSC_BITS_PER_PIXEL_1_16:
> + return 16;
> + case DP_DSC_BITS_PER_PIXEL_1_8:
> + return 8;
> + case DP_DSC_BITS_PER_PIXEL_1_4:
> + return 4;
> + case DP_DSC_BITS_PER_PIXEL_1_2:
> + return 2;
> + case DP_DSC_BITS_PER_PIXEL_1_1:
> + return 1;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_dp_dsc_sink_bpp_incr);
> +
> /**
> * drm_dp_dsc_sink_max_slice_count() - Get the max slice count
> * supported by the DSC sink.
> diff --git a/include/drm/display/drm_dp_helper.h
> b/include/drm/display/drm_dp_helper.h
> index 3369104e2d25..6968d4d87931 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -164,6 +164,7 @@ drm_dp_is_branch(const u8
> dpcd[DP_RECEIVER_CAP_SIZE]) }
>
> /* DP/eDP DSC support */
> +u8 drm_dp_dsc_sink_bpp_incr(const u8
> +dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> u8 drm_dp_dsc_sink_max_slice_count(const u8
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
> bool is_edp);
> u8 drm_dp_dsc_sink_line_buf_depth(const u8
> dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
> --
> 2.25.1
RE: [PATCH v7 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters
> -Original Message-
> From: Dmitry Baryshkov
> Sent: Wednesday, May 17, 2023 3:58 PM
> To: David Airlie ; Daniel Vetter ; Jani
> Nikula ; Kandpal, Suraj
> ; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Tvrtko Ursulin ; Rob Clark
> ; Abhinav Kumar ;
> Sean Paul ; Marijn Suijten
>
> Cc: Ville Syrjälä ; dri-
> de...@lists.freedesktop.org; intel-...@lists.freedesktop.org; linux-arm-
> m...@vger.kernel.org; freedr...@lists.freedesktop.org
> Subject: [PATCH v7 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR)
> parameters
>
> The array of rc_parameters contains a mixture of parameters from DSC 1.1
> and DSC 1.2 standards. Split these tow configuration arrays in preparation to
> adding more configuration data.
>
LGTM.
Reviewed-by: Suraj Kandpal
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 139 ++
> drivers/gpu/drm/i915/display/intel_vdsc.c | 10 +-
> include/drm/display/drm_dsc_helper.h | 7 +-
> 3 files changed, 129 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index acb93d4116e0..f1ba39df5708 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -325,10 +325,88 @@ struct rc_parameters_data {
> #define DSC_BPP(bpp) ((bpp) << 4)
>
> /*
> - * Selected Rate Control Related Parameter Recommended Values
> - * from DSC_v1.11 spec & C Model release: DSC_model_20161212
> + * Rate Control Related Parameter Recommended Values from DSC_v1.1
> spec
> + prior
> + * to DSC 1.1 fractional bpp underflow SCR (DSC_v1.1_E1.pdf)
> + *
> + * Cross-checked against C Model releases: DSC_model_20161212 and
> + 20210623
> */
> -static const struct rc_parameters_data rc_parameters[] = {
> +static const struct rc_parameters_data rc_parameters_pre_scr[] = {
> + {
> + .bpp = DSC_BPP(8), .bpc = 8,
> + { 512, 12, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12,
> -12 },
> + { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 10,
> + { 512, 12, 6144, 7, 16, 15, 15, {
> + /*
> + * DSC model/pre-SCR-cfg has 8 for
> range_max_qp[0], however
> + * VESA DSC 1.1 Table E-5 sets it to 4.
> + */
> + { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12,
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 12,
> + { 512, 12, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 8,
> + { 341, 15, 2048, 3, 12, 11, 11, {
> + { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
> + { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13,
> 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 10,
> + { 341, 15, 2048, 7, 16, 15, 15, {
> + { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
> + { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8
> },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 12,
> +
RE: [PATCH v7 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters
>
> The array of rc_parameters contains a mixture of parameters from DSC 1.1
> and DSC 1.2 standards. Split these tow configuration arrays in preparation to
> adding more configuration data.
>
> Signed-off-by: Dmitry Baryshkov
LGTM.
Reviewed-by: Suraj Kandpal
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 139 ++
> drivers/gpu/drm/i915/display/intel_vdsc.c | 10 +-
> include/drm/display/drm_dsc_helper.h | 7 +-
> 3 files changed, 129 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index acb93d4116e0..f1ba39df5708 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -325,10 +325,88 @@ struct rc_parameters_data {
> #define DSC_BPP(bpp) ((bpp) << 4)
>
> /*
> - * Selected Rate Control Related Parameter Recommended Values
> - * from DSC_v1.11 spec & C Model release: DSC_model_20161212
> + * Rate Control Related Parameter Recommended Values from DSC_v1.1
> spec
> + prior
> + * to DSC 1.1 fractional bpp underflow SCR (DSC_v1.1_E1.pdf)
> + *
> + * Cross-checked against C Model releases: DSC_model_20161212 and
> + 20210623
> */
> -static const struct rc_parameters_data rc_parameters[] = {
> +static const struct rc_parameters_data rc_parameters_pre_scr[] = {
> + {
> + .bpp = DSC_BPP(8), .bpc = 8,
> + { 512, 12, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12,
> -12 },
> + { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 10,
> + { 512, 12, 6144, 7, 16, 15, 15, {
> + /*
> + * DSC model/pre-SCR-cfg has 8 for
> range_max_qp[0], however
> + * VESA DSC 1.1 Table E-5 sets it to 4.
> + */
> + { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12,
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 12,
> + { 512, 12, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 8,
> + { 341, 15, 2048, 3, 12, 11, 11, {
> + { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
> + { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13,
> 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 10,
> + { 341, 15, 2048, 7, 16, 15, 15, {
> + { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
> + { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8
> },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 12,
> + { 341, 15, 2048, 11, 20, 19, 19, {
> + { 0, 6, 2 }, { 4, 9, 0 }, { 7, 11, 0 }, { 8, 12, -2 },
> + { 10, 13, -4 }, { 11, 14, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + { /* sentinel */ }
> +};
> +
> +/*
> + * Selected Rate Control Related Parameter Recommended Values from DSC
> +v1.2, v1.2a, v1.2b and
> + * DSC_v1.1_E1 specs.
> + *
> + * Cross-checked against C Model releases: DSC_model_20161212 and
> +20210623 */ static const struct rc_parameters_data
> +rc_parameters_1_2_444[] = {
> {
> .bpp = DSC_BPP(6), .bpc = 8,
> { 768, 15, 6144, 3, 13, 11, 11, {
> @@ -388,22 +466,18 @@
RE: [Freedreno] [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters
> -Original Message-
> From: Dmitry Baryshkov
> Sent: Wednesday, May 17, 2023 5:33 AM
> To: Kandpal, Suraj ; David Airlie
> ; Daniel Vetter ; Jani Nikula
> ; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Tvrtko Ursulin ; Rob Clark
> ; Abhinav Kumar ;
> Sean Paul ; Marijn Suijten
>
> Cc: linux-arm-...@vger.kernel.org; intel-...@lists.freedesktop.org;
> freedr...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Ville
> Syrjälä
> Subject: Re: [Freedreno] [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and
> DSC 1.1 (pre-SCR) parameters
>
> On 16/05/2023 21:46, Kandpal, Suraj wrote:
> >>
> >> The array of rc_parameters contains a mixture of parameters from DSC
> >> 1.1 and DSC 1.2 standards. Split these tow configuration arrays in
> >> preparation to adding more configuration data.
> >>
> >
> > Hi ,
> > Needed to add some more comments apart from the previous ones
> already
> > given
> >
>
> [skipped]
>
>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> index d4340b18c18d..bd9116d2cd76 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> @@ -226,7 +226,15 @@ int intel_dsc_compute_params(struct
> >> intel_crtc_state *pipe_config)
> >>if (DISPLAY_VER(dev_priv) >= 13) {
> >>calculate_rc_params(vdsc_cfg);
> >>} else {
> >> - ret = drm_dsc_setup_rc_params(vdsc_cfg);
> >> + if ((compressed_bpp == 8 ||
> >> + compressed_bpp == 12) &&
> >> + (vdsc_cfg->bits_per_component == 8 ||
> >> + vdsc_cfg->bits_per_component == 10 ||
> >> + vdsc_cfg->bits_per_component == 12))
> >> + ret = drm_dsc_setup_rc_params(vdsc_cfg,
> >> DRM_DSC_1_1_PRE_SCR);
> >> + else
> >> + ret = drm_dsc_setup_rc_params(vdsc_cfg,
> >> DRM_DSC_1_2_444);
> >> +
> >
> > I do not think this kind of assignment works as you will also be
> > adding
> > DRM_DSC_1_2_422 and DRM_DSC_1_2_420 in further patches and AFAICS
> > There is no where in patch 8 that you have accounted for when 422 or 420
> will be used.
> > Maybe you can add an if case inside the else block to check
> > pipe_config->output_format to pass the rc_param_data in patch 8
>
> I don't think this is necessary for now. The driver doesn't support YUV 422.
> The YUV 420 is supported only for DISPLAY_VER(dev_priv) >= 14, however
> these helpers are only used for DISPLAY_VER(dev_priv) < 13.
>
> I did not move RC calculation to drm_dsc_helpers.c (yet ?), which is used for
> DISPLAY_VER >= 13.
Hmm. I see I'll work on it once this patch series is merged
Regards,
Suraj Kandpal
>
> >
> > Regards,
> > Suraj Kandpal
> >>if (ret)
> >>return ret;
> >>
> >> diff --git a/include/drm/display/drm_dsc_helper.h
> >> b/include/drm/display/drm_dsc_helper.h
> >> index 1681791f65a5..c634bb2935d3 100644
> >> --- a/include/drm/display/drm_dsc_helper.h
> >> +++ b/include/drm/display/drm_dsc_helper.h
> >> @@ -10,12 +10,17 @@
> >>
> >> #include
> >>
> >> +enum drm_dsc_params_kind {
> >> + DRM_DSC_1_2_444,
> >> + DRM_DSC_1_1_PRE_SCR, /* legacy params from DSC 1.1 */ };
> >> +
> >> void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header);
> >> int
> >> drm_dsc_dp_rc_buffer_size(u8 rc_buffer_block_size, u8
> >> rc_buffer_size); void drm_dsc_pps_payload_pack(struct
> >> drm_dsc_picture_parameter_set *pps_sdp,
> >> const struct drm_dsc_config *dsc_cfg); void
> >> drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg); -int
> >> drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg);
> >> +int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum
> >> +drm_dsc_params_kind kind);
> >> int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
> >>
> >> #endif /* _DRM_DSC_HELPER_H_ */
> >> --
> >> 2.39.2
> >
>
> --
> With best wishes
> Dmitry
RE: [PATCH v6 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters
>
> The array of rc_parameters contains a mixture of parameters from DSC 1.1
> and DSC 1.2 standards. Split these tow configuration arrays in preparation to
> adding more configuration data.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 139 ++
> drivers/gpu/drm/i915/display/intel_vdsc.c | 10 +-
> include/drm/display/drm_dsc_helper.h | 7 +-
> 3 files changed, 129 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index acb93d4116e0..f9d01d72c1ff 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -325,10 +325,88 @@ struct rc_parameters_data {
> #define DSC_BPP(bpp) ((bpp) << 4)
>
> /*
> - * Selected Rate Control Related Parameter Recommended Values
> - * from DSC_v1.11 spec & C Model release: DSC_model_20161212
> + * Rate Control Related Parameter Recommended Values from DSC_v1.1
> spec
> + prior
> + * to DSC 1.1 fractional bpp underflow SCR (DSC_v1.1_E1.pdf)
> + *
> + * Cross-checked against C Model releases: DSC_model_20161212 and
> + 20210623
> */
> -static const struct rc_parameters_data rc_parameters[] = {
> +static const struct rc_parameters_data rc_parameters_pre_scr[] = {
> + {
> + .bpp = DSC_BPP(8), .bpc = 8,
> + { 512, 12, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12,
> -12 },
> + { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 10,
> + { 512, 12, 6144, 7, 16, 15, 15, {
> + /*
> + * DSC model/pre-SCR-cfg has 8 for
> range_max_qp[0], however
> + * VESA DSC 1.1 Table E-5 sets it to 4.
> + */
> + { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12,
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 12,
> + { 512, 12, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 8,
> + { 341, 15, 2048, 3, 12, 11, 11, {
> + { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
> + { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13,
> 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 10,
> + { 341, 15, 2048, 7, 16, 15, 15, {
> + { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
> + { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8
> },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 12,
> + { 341, 15, 2048, 11, 20, 19, 19, {
> + { 0, 6, 2 }, { 4, 9, 0 }, { 7, 11, 0 }, { 8, 12, -2 },
> + { 10, 13, -4 }, { 11, 14, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + { /* sentinel */ }
> +};
> +
> +/*
> + * Selected Rate Control Related Parameter Recommended Values from DSC
> +v1.2, v1.2a, v1.2b and
> + * DSC_v1.1_E1 specs.
> + *
> + * Cross-checked against C Model releases: DSC_model_20161212 and
> +20210623 */ static const struct rc_parameters_data
> +rc_parameters_1_2_444[] = {
> {
> .bpp = DSC_BPP(6), .bpc = 8,
> { 768, 15, 6144, 3, 13, 11, 11, {
> @@ -388,22 +466,18 @@ static const struct
RE: [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters
>
> The array of rc_parameters contains a mixture of parameters from DSC 1.1
> and DSC 1.2 standards. Split these tow configuration arrays in preparation to
> adding more configuration data.
>
Hi ,
Needed to add some more comments apart from the previous ones already given
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 127 ++
> drivers/gpu/drm/i915/display/intel_vdsc.c | 10 +-
> include/drm/display/drm_dsc_helper.h | 7 +-
> 3 files changed, 119 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index acb93d4116e0..35b39f3109c4 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -324,11 +324,81 @@ struct rc_parameters_data {
>
> #define DSC_BPP(bpp) ((bpp) << 4)
>
> +static const struct rc_parameters_data rc_parameters_pre_scr[] = {
> + {
> + .bpp = DSC_BPP(8), .bpc = 8,
> + { 512, 12, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12,
> -12 },
> + { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 10,
> + { 512, 12, 6144, 7, 16, 15, 15, {
> + /*
> + * DSC model/pre-SCR-cfg has 8 for
> range_max_qp[0], however
> + * VESA DSC 1.1 Table E-5 sets it to 4.
> + */
> + { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12,
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 12,
> + { 512, 12, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 8,
> + { 341, 15, 2048, 3, 12, 11, 11, {
> + { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
> + { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13,
> 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 10,
> + { 341, 15, 2048, 7, 16, 15, 15, {
> + { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
> + { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8
> },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 12,
> + { 341, 15, 2048, 11, 20, 19, 19, {
> + { 0, 6, 2 }, { 4, 9, 0 }, { 7, 11, 0 }, { 8, 12, -2 },
> + { 10, 13, -4 }, { 11, 14, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + { /* sentinel */ }
> +};
> +
> /*
> * Selected Rate Control Related Parameter Recommended Values
> * from DSC_v1.11 spec & C Model release: DSC_model_20161212
> */
> -static const struct rc_parameters_data rc_parameters[] = {
> +static const struct rc_parameters_data rc_parameters_1_2_444[] = {
> {
> .bpp = DSC_BPP(6), .bpc = 8,
> { 768, 15, 6144, 3, 13, 11, 11, {
> @@ -388,22 +458,18 @@ static const struct rc_parameters_data
> rc_parameters[] = {
> { 512, 12, 6144, 3, 12, 11, 11, {
> { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> - { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12,
> -12 },
> - { 5, 13, -12 }, { 7, 13, -12
RE: [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters
> Subject: [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR)
> parameters
>
> The array of rc_parameters contains a mixture of parameters from DSC 1.1
> and DSC 1.2 standards. Split these tow configuration arrays in preparation to
> adding more configuration data.
>
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 127 ++
> drivers/gpu/drm/i915/display/intel_vdsc.c | 10 +-
> include/drm/display/drm_dsc_helper.h | 7 +-
> 3 files changed, 119 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index acb93d4116e0..35b39f3109c4 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -324,11 +324,81 @@ struct rc_parameters_data {
>
> #define DSC_BPP(bpp) ((bpp) << 4)
>
Maybe comment here mentioning the DSC version and the C Model
we follow would be useful
> +static const struct rc_parameters_data rc_parameters_pre_scr[] = {
> + {
> + .bpp = DSC_BPP(8), .bpc = 8,
> + { 512, 12, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12,
> -12 },
> + { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 10,
> + { 512, 12, 6144, 7, 16, 15, 15, {
> + /*
> + * DSC model/pre-SCR-cfg has 8 for
> range_max_qp[0], however
> + * VESA DSC 1.1 Table E-5 sets it to 4.
> + */
> + { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12,
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 12,
> + { 512, 12, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 8,
> + { 341, 15, 2048, 3, 12, 11, 11, {
> + { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
> + { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13,
> 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 10,
> + { 341, 15, 2048, 7, 16, 15, 15, {
> + { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
> + { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8
> },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16,
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 12,
> + { 341, 15, 2048, 11, 20, 19, 19, {
> + { 0, 6, 2 }, { 4, 9, 0 }, { 7, 11, 0 }, { 8, 12, -2 },
> + { 10, 13, -4 }, { 11, 14, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + { /* sentinel */ }
> +};
> +
> /*
> * Selected Rate Control Related Parameter Recommended Values
> * from DSC_v1.11 spec & C Model release: DSC_model_20161212
> */
The above comment shouldn't be above this function anymore since
This represent dsc_v1.2 I presume maybe move this comment above
and add a new comment for this function.
> -static const struct rc_parameters_data rc_parameters[] = {
> +static const struct rc_parameters_data rc_parameters_1_2_444[] = {
> {
> .bpp = DSC_BPP(6), .bpc = 8,
> { 768, 15, 6144, 3, 13, 11, 11, {
> @@ -388,22 +458,18 @@ static const struct rc_parameters_data
> rc_parameters[] = {
> { 512, 12, 6144, 3, 12, 11, 11, {
> {
RE: [PATCH v9 07/10] drm/i915/hdcp: Use HDCP helpers for i915
> -Original Message-
> From: Mark Yacoub
> Sent: Wednesday, April 12, 2023 12:52 AM
> To: Jani Nikula ; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Tvrtko Ursulin ; David Airlie
> ; Daniel Vetter
> Cc: seanp...@chromium.org; Kandpal, Suraj ;
> diand...@chromium.org; dmitry.barysh...@linaro.org; dri-
> de...@lists.freedesktop.org; freedr...@lists.freedesktop.org; intel-
> g...@lists.freedesktop.org; Nikula, Jani ; Mark Yacoub
> ; linux-ker...@vger.kernel.org
> Subject: [PATCH v9 07/10] drm/i915/hdcp: Use HDCP helpers for i915
>
> From: Sean Paul
>
> Now that all of the HDCP 1.x logic has been migrated to the central HDCP
> helpers, use it in the i915 driver.
>
> The majority of the driver code for HDCP 1.x will live in intel_hdcp.c,
> however there are a few helper hooks which are connector-specific and
> need to be partially or fully implemented in the intel_dp_hdcp.c or
> intel_hdmi.c.
>
> We'll leave most of the HDCP 2.x code alone since we don't have another
> implementation of HDCP 2.x to use as reference for what should and
> should not live in the drm helpers. The helper will call the overly
> general enable/disable/is_capable HDCP 2.x callbacks and leave the
> interesting stuff for the driver. Once we have another HDCP 2.x
> implementation, we should do a similar migration.
>
> Acked-by: Jani Nikula
> Acked-by: Rodrigo Vivi
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
>
> ---
> Changes in v2:
> -Fix mst helper function pointer reported by 0-day
> Changes in v3:
> -Add forward declaration for drm_atomic_state in intel_hdcp.h identified
> by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased.
> Changes in v7:
> -Added to drm_hdcp_helper_funcs new functions that are unique between
> DP
> and HDMI
> -Adjusted the function signatures to take "driver data"
> Changes in v8:
> -None
> Changes in v9:
> -rename dev_priv to i915
> -remove display type specific hdcp calls init from driver
>
> drivers/gpu/drm/i915/display/intel_ddi.c | 32 +-
> .../drm/i915/display/intel_display_debugfs.c | 7 +-
> .../drm/i915/display/intel_display_types.h| 51 +-
> drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 352 +++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 984 --
> drivers/gpu/drm/i915/display/intel_hdcp.h | 43 +-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 267 ++---
> 8 files changed, 475 insertions(+), 1277 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 254559abedfba..8a2f20c929e9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -28,6 +28,7 @@
> #include
>
> #include
> +#include
> #include
>
> #include "i915_drv.h"
> @@ -2956,6 +2957,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
>const struct intel_crtc_state *crtc_state,
>const struct drm_connector_state *conn_state)
> {
> + struct intel_connector *connector =
> + to_intel_connector(conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
> drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
>
> if (!intel_crtc_is_bigjoiner_slave(crtc_state))
> @@ -2975,12 +2980,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
> else
> intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
>
> - /* Enable hdcp if it's desired */
> - if (conn_state->content_protection ==
> - DRM_MODE_CONTENT_PROTECTION_DESIRED)
> - intel_hdcp_enable(to_intel_connector(conn_state-
> >connector),
> - crtc_state,
> - (u8)conn_state->hdcp_content_type);
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> + >base,
> + _port->hdcp_mutex);
> }
>
> static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> @@ -3026,7 +3029,14 @@ static void intel_disable_ddi(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state
> *old_conn_state)
> {
> - intel_hdcp_disable(to_intel_connector(old_conn_state->con
RE: [PATCH v9 02/10] drm/hdcp: Avoid changing crtc state in hdcp atomic check
>
> From: Sean Paul
>
> Instead of forcing a modeset in the hdcp atomic check, rename to
> drm_hdcp_has_changed and return true if the content protection value is
> changing and let the driver decide whether a modeset is required or not.
>
> Acked-by: Jani Nikula
> Reviewed-by: Rodrigo Vivi
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
>
> ---
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebase: modifications in drm_hdcp_helper.c instead of drm_hdcp.c
> Changes in v7:
> -Renamed the function from drm_hdcp_atomic_check to
> drm_hdcp_has_changed (Dmitry Baryshkov)
>
> drivers/gpu/drm/display/drm_hdcp_helper.c | 39 ++---
> drivers/gpu/drm/i915/display/intel_atomic.c | 6 ++--
> include/drm/display/drm_hdcp_helper.h | 2 +-
> 3 files changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdcp_helper.c
> b/drivers/gpu/drm/display/drm_hdcp_helper.c
> index 7ca390b3ea106..34baf2b97cd87 100644
> --- a/drivers/gpu/drm/display/drm_hdcp_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdcp_helper.c
> @@ -422,18 +422,21 @@ void drm_hdcp_update_content_protection(struct
> drm_connector *connector,
> EXPORT_SYMBOL(drm_hdcp_update_content_protection);
>
> /**
> - * drm_hdcp_atomic_check - Helper for drivers to call during connector-
> >atomic_check
> + * drm_hdcp_has_changed - Helper for drivers to call during
> + connector->atomic_check
> *
> * @state: pointer to the atomic state being checked
> * @connector: drm_connector on which content protection state needs an
> update
> *
> * This function can be used by display drivers to perform an atomic check
> on the
> - * hdcp state elements. If hdcp state has changed, this function will set
> - * mode_changed on the crtc driving the connector so it can update its
> hardware
> - * to match the hdcp state.
> + * hdcp state elements. If hdcp state has changed in a manner which
> + requires the
> + * driver to enable or disable content protection, this function will
> + return
> + * true.
> + *
> + * Returns:
> + * true if the driver must enable/disable hdcp, false otherwise
> */
> -void drm_hdcp_atomic_check(struct drm_connector *connector,
> -struct drm_atomic_state *state)
> +bool drm_hdcp_has_changed(struct drm_connector *connector,
> + struct drm_atomic_state *state)
> {
> struct drm_connector_state *new_conn_state, *old_conn_state;
> struct drm_crtc_state *new_crtc_state; @@ -450,19 +453,31 @@
> void drm_hdcp_atomic_check(struct drm_connector *connector,
>* If the connector is being disabled with CP enabled, mark it
>* desired so it's re-enabled when the connector is brought
> back
>*/
> - if (old_hdcp ==
> DRM_MODE_CONTENT_PROTECTION_ENABLED)
> + if (old_hdcp ==
> DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> new_conn_state->content_protection =
>
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> - return;
> + return true;
> + }
> + return false;
> }
>
> new_crtc_state =
> drm_atomic_get_new_crtc_state(state, new_conn_state-
> >crtc);
> if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> - new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> + new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> {
> new_conn_state->content_protection =
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return true;
> + }
> +
> + /*
> + * Coming back from UNDESIRED state, CRTC change or re-
> enablement requires
> + * the driver to try CP enable.
> + */
> + if (new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> + new_conn_state->crtc != old_conn_state->crtc)
> + return true;
>
> /*
>* Nothing to do if content type is unchanged and one of:
> @@ -477,9 +492,9 @@ void drm_hdcp_atomic_check(struct drm_connector
> *connector,
>new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> if (old_conn_state->hdcp_content_type ==
> new_conn_state->hdcp_content_type)
> - return;
> + return false;
> }
>
> - new_crtc_state->mode_changed = true;
> + return true;
> }
So previously in hdcp_atomic_check we decided if a mode change was required
only after going through two different checks
if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
(old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
and
if (old_hdcp == new_hdcp ||
(old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
RE: [PATCH v9 01/10] drm/hdcp: Add drm_hdcp_atomic_check()
Yacoub
> ; linux-ker...@vger.kernel.org
> Subject: [PATCH v9 01/10] drm/hdcp: Add drm_hdcp_atomic_check()
>
> From: Sean Paul
>
> Move the hdcp atomic check from i915 to drm_hdcp so other drivers can use
> it. No functional changes, just cleaned up some of the code when moving it
> over.
>
> Acked-by: Jani Nikula
> Reviewed-by: Rodrigo Vivi
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
>
> ---
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebase: move helper from drm_hdcp.c to drm_hdcp_helper.c Changes in
> v7:
> -Removed links to patch from commit msg (Dmitry Baryshkov)
>
> drivers/gpu/drm/display/drm_hdcp_helper.c | 64
> +
> drivers/gpu/drm/i915/display/intel_atomic.c | 4 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 47 ---
> drivers/gpu/drm/i915/display/intel_hdcp.h | 3 -
> include/drm/display/drm_hdcp_helper.h | 3 +
> 5 files changed, 69 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdcp_helper.c
> b/drivers/gpu/drm/display/drm_hdcp_helper.c
> index e78999c72bd77..7ca390b3ea106 100644
> --- a/drivers/gpu/drm/display/drm_hdcp_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdcp_helper.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> static inline void drm_hdcp_print_ksv(const u8 *ksv) { @@ -419,3 +420,66
> @@ void drm_hdcp_update_content_protection(struct drm_connector
> *connector,
>dev-
> >mode_config.content_protection_property);
> }
> EXPORT_SYMBOL(drm_hdcp_update_content_protection);
> +
> +/**
> + * drm_hdcp_atomic_check - Helper for drivers to call during
> +connector->atomic_check
> + *
> + * @state: pointer to the atomic state being checked
> + * @connector: drm_connector on which content protection state needs an
> +update
> + *
> + * This function can be used by display drivers to perform an atomic
> +check on the
> + * hdcp state elements. If hdcp state has changed, this function will
> +set
> + * mode_changed on the crtc driving the connector so it can update its
> +hardware
> + * to match the hdcp state.
> + */
> +void drm_hdcp_atomic_check(struct drm_connector *connector,
> +struct drm_atomic_state *state)
> +{
> + struct drm_connector_state *new_conn_state, *old_conn_state;
> + struct drm_crtc_state *new_crtc_state;
> + u64 old_hdcp, new_hdcp;
> +
> + old_conn_state = drm_atomic_get_old_connector_state(state,
> connector);
> + old_hdcp = old_conn_state->content_protection;
> +
> + new_conn_state = drm_atomic_get_new_connector_state(state,
> connector);
> + new_hdcp = new_conn_state->content_protection;
> +
Nit: Why not assign these as right when they are declared we would end up using
less lines
> + if (!new_conn_state->crtc) {
> + /*
> + * If the connector is being disabled with CP enabled, mark it
> + * desired so it's re-enabled when the connector is brought
> back
> + */
> + if (old_hdcp ==
> DRM_MODE_CONTENT_PROTECTION_ENABLED)
> + new_conn_state->content_protection =
> +
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return;
> + }
> +
> + new_crtc_state =
> + drm_atomic_get_new_crtc_state(state, new_conn_state-
> >crtc);
> + if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> + new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +
> + /*
> + * Nothing to do if content type is unchanged and one of:
> + * - state didn't change
> + * - HDCP was activated since the last commit
> + * - attempting to set to desired while already enabled
> + */
> + if (old_hdcp == new_hdcp ||
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> + new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> + new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> + if (old_conn_state->hdcp_content_type ==
> + new_conn_state->hdcp_content_type)
> + return;
> + }
> +
> + new_crtc_state->mode_changed = true;
> +}
> +EXPORT_SYMBOL(drm_hdcp_atomic_check);
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index a9a3f3715279d..e9d00b6a63d39 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
>
> #include "i915_drv.h"
> #include "i915_reg.h"
> @@ -39,7 +40,6 @@
> #include "intel_cdclk.h"
> #include
RE: [PATCH v4 08/12] drm/display/dsc: add YCbCr 4:2:2 and 4:2:0 RC parameters
Hi,
> Include RC parameters for YCbCr 4:2:2 and 4:2:0 configurations.
>
Looks Good to me
Reviewed-by: Suraj Kandpal
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 438
> +++
> include/drm/display/drm_dsc_helper.h | 2 +
> 2 files changed, 440 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index aec6f8c201af..65e810a54257 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -740,6 +740,438 @@ static const struct rc_parameters_data
> rc_parameters_1_2_444[] = {
> { /* sentinel */ }
> };
>
> +static const struct rc_parameters_data rc_parameters_1_2_422[] = {
> + {
> + .bpp = DSC_BPP(6), .bpc = 8,
> + { 512, 15, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 10, -10 }, { 5, 11,
> -12 },
> + { 5, 11, -12 }, { 9, 12, -12 }, { 12, 13, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(6), .bpc = 10,
> + { 512, 15, 6144, 7, 16, 15, 15, {
> + { 0, 8, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12,
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 14, -10 }, { 9, 15,
> -12 },
> + { 9, 15, -12 }, { 13, 16, -12 }, { 16, 17, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(6), .bpc = 12,
> + { 512, 15, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 18, -10 },
> + { 13, 19, -12 }, { 13, 19, -12 }, { 17, 20, -12 },
> + { 20, 21, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(6), .bpc = 14,
> + { 512, 15, 6144, 15, 24, 23, 23, {
> + { 0, 12, 2 }, { 5, 13, 0 }, { 11, 15, 0 }, { 12, 17, -2
> },
> + { 15, 19, -4 }, { 15, 19, -6 }, { 15, 19, -8 }, { 15,
> 20, -8 },
> + { 15, 21, -8 }, { 15, 22, -10 }, { 17, 22, -10 },
> + { 17, 23, -12 }, { 17, 23, -12 }, { 21, 24, -12 },
> + { 24, 25, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(6), .bpc = 16,
> + { 512, 15, 6144, 19, 28, 27, 27, {
> + { 0, 12, 2 }, { 6, 14, 0 }, { 13, 17, 0 }, { 15, 20, -2
> },
> + { 19, 23, -4 }, { 19, 23, -6 }, { 19, 23, -8 }, { 19,
> 24, -8 },
> + { 19, 25, -8 }, { 19, 26, -10 }, { 21, 26, -10 },
> + { 21, 27, -12 }, { 21, 27, -12 }, { 25, 28, -12 },
> + { 28, 29, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(7), .bpc = 8,
> + { 410, 15, 5632, 3, 12, 11, 11, {
> + { 0, 3, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 2, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 9, -10 }, { 5, 10, -10 }, { 5, 10,
> -10 },
> + { 5, 11, -12 }, { 7, 11, -12 }, { 11, 12, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(7), .bpc = 10,
> + { 410, 15, 5632, 7, 16, 15, 15, {
> + { 0, 7, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 6, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12,
> -8 },
> + { 7, 13, -8 }, { 7, 13, -10 }, { 9, 14, -10 }, { 9, 14,
> -10 },
> + { 9, 15, -12 }, { 11, 15, -12 }, { 15, 16, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(7), .bpc = 12,
> + { 410, 15, 5632, 11, 20, 19, 19, {
> + { 0, 11, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 10, 14, -2
> },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11,
> 16, -8 },
> + { 11, 17, -8 }, { 11, 17, -10 }, { 13, 18, -10 },
> + { 13, 18, -10 }, { 13, 19, -12 }, { 15, 19, -12 },
> + { 19, 20, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(7), .bpc = 14,
> + { 410, 15, 5632, 15, 24, 23, 23, {
> + { 0, 11, 2 }, { 5, 13, 0 }, { 11, 15, 0 }, { 13, 18, -2
>
RE: [PATCH 0/7] Enable YCbCr420 format for VDSC
Hi Dmitry > -Original Message- > From: Dmitry Baryshkov > Sent: Friday, April 7, 2023 8:28 AM > To: Kandpal, Suraj ; Jani Nikula > ; dri-devel@lists.freedesktop.org; intel- > g...@lists.freedesktop.org > Cc: Nautiyal, Ankit K ; Shankar, Uma > ; Maarten Lankhorst > > Subject: Re: [PATCH 0/7] Enable YCbCr420 format for VDSC > > Hi Suraj > > On 28/03/2023 16:20, Kandpal, Suraj wrote: > > > > > >> -Original Message- > >> From: dri-devel On Behalf > >> Of Jani Nikula > >> Sent: Wednesday, March 8, 2023 5:00 PM > >> To: Kandpal, Suraj ; dri- > >> de...@lists.freedesktop.org; intel-...@lists.freedesktop.org > >> Cc: Dmitry Baryshkov ; Nautiyal, Ankit K > >> ; Shankar, Uma ; > >> Kandpal, Suraj > >> Subject: Re: [PATCH 0/7] Enable YCbCr420 format for VDSC > >> > >> On Wed, 22 Feb 2023, Suraj Kandpal wrote: > >>> This patch series aims to enable the YCbCr420 format for DSC. > >>> Changes are mostly compute params related for hdmi,dp and dsi along > >>> with the addition of new rc_tables for native_420 and corresponding > >>> changes to macros used to fetch them. > >>> There have been discussions prior to this series in which some > >>> patches have gotten rb and can be found in the below link > >>> https://patchwork.freedesktop.org/series/113729 > >> > >> I think it would be useful to get [1] from Dmitry merged to > >> drm-misc-next first, have that in drm-next, and again backmerged to > >> drm-intel-next before this. At least patches 1-5. > >> > >> There's not much point in all drivers duplicating the parameters, and > >> we need to move towards common code. Dmitry has been helpful in > >> contributing this to us. > >> > >> BR, > >> Jani. > >> > >> > > > > Hi Jani, > > Maarten has acked the patch series to be merged through drm-intel and > > in the meantime I will work with Dmitry to pull the common code to > > avoid duplication > > I wanted to check, are there any updates from your side regarding the series > at [1] ? > Will have a look and float comments if any by Monday > > > > Regards, > > Suraj Kandpal > > > >> [1] https://patchwork.freedesktop.org/series/114473/ > > > > -- > With best wishes > Dmitry Regards, Suraj Kandpal
RE: [PATCH v8 07/10] drm/i915/hdcp: Use HDCP helpers for i915
> -Original Message-
> From: Mark Yacoub
> Sent: Saturday, April 1, 2023 3:42 AM
> To: Jani Nikula ; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Tvrtko Ursulin ; David Airlie
> ; Daniel Vetter
> Cc: seanp...@chromium.org; Kandpal, Suraj ;
> diand...@chromium.org; dri-devel@lists.freedesktop.org;
> freedr...@lists.freedesktop.org; intel-...@lists.freedesktop.org; Nikula, Jani
> ; Mark Yacoub ; linux-
> ker...@vger.kernel.org
> Subject: [PATCH v8 07/10] drm/i915/hdcp: Use HDCP helpers for i915
>
> From: Sean Paul
>
> Now that all of the HDCP 1.x logic has been migrated to the central HDCP
> helpers, use it in the i915 driver.
>
> The majority of the driver code for HDCP 1.x will live in intel_hdcp.c,
> however there are a few helper hooks which are connector-specific and
> need to be partially or fully implemented in the intel_dp_hdcp.c or
> intel_hdmi.c.
>
> We'll leave most of the HDCP 2.x code alone since we don't have another
> implementation of HDCP 2.x to use as reference for what should and
> should not live in the drm helpers. The helper will call the overly
> general enable/disable/is_capable HDCP 2.x callbacks and leave the
> interesting stuff for the driver. Once we have another HDCP 2.x
> implementation, we should do a similar migration.
>
> Acked-by: Jani Nikula
> Acked-by: Rodrigo Vivi
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
>
> ---
> Changes in v2:
> -Fix mst helper function pointer reported by 0-day
> Changes in v3:
> -Add forward declaration for drm_atomic_state in intel_hdcp.h identified
> by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased.
> Changes in v7:
> -Added to drm_hdcp_helper_funcs new functions that are unique between
> DP
> and HDMI
> -Adjusted the function signatures to take "driver data"
> Changes in v8:
> -None
>
> drivers/gpu/drm/i915/display/intel_ddi.c | 32 +-
> .../drm/i915/display/intel_display_debugfs.c | 6 +-
> .../drm/i915/display/intel_display_types.h| 51 +-
> drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 368 +++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 960 --
> drivers/gpu/drm/i915/display/intel_hdcp.h | 37 +-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 276 ++---
> 8 files changed, 484 insertions(+), 1262 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 254559abedfba..8a2f20c929e9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -28,6 +28,7 @@
> #include
>
> #include
> +#include
> #include
>
> #include "i915_drv.h"
> @@ -2956,6 +2957,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
>const struct intel_crtc_state *crtc_state,
>const struct drm_connector_state *conn_state)
> {
> + struct intel_connector *connector =
> + to_intel_connector(conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
> drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
>
> if (!intel_crtc_is_bigjoiner_slave(crtc_state))
> @@ -2975,12 +2980,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
> else
> intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
>
> - /* Enable hdcp if it's desired */
> - if (conn_state->content_protection ==
> - DRM_MODE_CONTENT_PROTECTION_DESIRED)
> - intel_hdcp_enable(to_intel_connector(conn_state-
> >connector),
> - crtc_state,
> - (u8)conn_state->hdcp_content_type);
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> + >base,
> + _port->hdcp_mutex);
> }
>
> static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> @@ -3026,7 +3029,14 @@ static void intel_disable_ddi(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state
> *old_conn_state)
> {
> - intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
> + struct intel_connector *connector =
> + to_intel_connector(old_conn_state->connector);
> + struct inte
RE: [PATCH v8 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
> -Original Message-
> From: Kandpal, Suraj
> Sent: Monday, April 3, 2023 12:12 PM
> To: Mark Yacoub ; Jani Nikula
> ; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Tvrtko Ursulin ; David Airlie
> ; Daniel Vetter
> Cc: seanp...@chromium.org; diand...@chromium.org; dri-
> de...@lists.freedesktop.org; freedr...@lists.freedesktop.org; intel-
> g...@lists.freedesktop.org; Nikula, Jani ; linux-
> ker...@vger.kernel.org
> Subject: RE: [PATCH v8 06/10] drm/i915/hdcp: Retain hdcp_capable return
> codes
>
>
>
> > -Original Message-
> > From: Mark Yacoub
> > Sent: Saturday, April 1, 2023 3:42 AM
> > To: Jani Nikula ; Joonas Lahtinen
> > ; Vivi, Rodrigo
> > ; Tvrtko Ursulin
> > ; David Airlie ;
> > Daniel Vetter
> > Cc: seanp...@chromium.org; Kandpal, Suraj ;
> > diand...@chromium.org; dri-devel@lists.freedesktop.org;
> > freedr...@lists.freedesktop.org; intel-...@lists.freedesktop.org;
> > Nikula, Jani ; Mark Yacoub
> > ; linux- ker...@vger.kernel.org
> > Subject: [PATCH v8 06/10] drm/i915/hdcp: Retain hdcp_capable return
> > codes
> >
> > From: Sean Paul
> >
> > The shim functions return error codes, but they are discarded in
> > intel_hdcp.c. This patch plumbs the return codes through so they are
> > properly handled.
> >
> > Acked-by: Jani Nikula
> > Reviewed-by: Rodrigo Vivi
> > Signed-off-by: Sean Paul
> > Signed-off-by: Mark Yacoub
> >
> > ---
> > Changes in v2:
> > -None
> > Changes in v3:
> > -None
> > Changes in v4:
> > -None
> > Changes in v5:
> > -None
> > Changes in v6:
> > -Rebased
> > Changes in v7:
> > -None
> > Changes in v8:
> > -None
> >
> > .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> > drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++-
> > drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> > 3 files changed, 37 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 7bcd90384a46d..a14b86a07e545 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -494,6 +494,7 @@ static void intel_panel_info(struct seq_file *m,
> > static void intel_hdcp_info(struct seq_file *m,
> > struct intel_connector *intel_connector) {
> > + int ret;
> > bool hdcp_cap, hdcp2_cap;
> >
> > if (!intel_connector->hdcp.shim) {
> > @@ -501,8 +502,12 @@ static void intel_hdcp_info(struct seq_file *m,
> > goto out;
> > }
> >
> > - hdcp_cap = intel_hdcp_capable(intel_connector);
> > - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> > + ret = intel_hdcp_capable(intel_connector, _cap);
> > + if (ret)
> > + hdcp_cap = false;
> > + ret = intel_hdcp2_capable(intel_connector, _cap);
> > + if (ret)
> > + hdcp2_cap = false;
> >
> > if (hdcp_cap)
> > seq_puts(m, "HDCP1.4 ");
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 0a20bc41be55d..61a862ae1f286 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct
> > intel_digital_port *dig_port, }
> >
> > /* Is HDCP1.4 capable on Platform and Sink */ -bool
> > intel_hdcp_capable(struct intel_connector *connector)
> > +int intel_hdcp_capable(struct intel_connector *connector, bool
> > +*capable)
> > {
> > struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> > const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> > - bool capable = false;
> > u8 bksv[5];
> >
> > + *capable = false;
> > +
> > if (!shim)
> > - return capable;
> > + return 0;
> >
> > - if (shim->hdcp_capable) {
> > - shim->hdcp_capable(dig_port, );
> > - } else {
> > - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > - capable = true;
> > - }
> > + if (shim->hdcp_capable)
> > + return shim->hdcp_capable(dig_port, capable);
> > +
> > + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > +
RE: [PATCH v8 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
> -Original Message-
> From: Mark Yacoub
> Sent: Saturday, April 1, 2023 3:42 AM
> To: Jani Nikula ; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Tvrtko Ursulin ; David Airlie
> ; Daniel Vetter
> Cc: seanp...@chromium.org; Kandpal, Suraj ;
> diand...@chromium.org; dri-devel@lists.freedesktop.org;
> freedr...@lists.freedesktop.org; intel-...@lists.freedesktop.org; Nikula, Jani
> ; Mark Yacoub ; linux-
> ker...@vger.kernel.org
> Subject: [PATCH v8 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
>
> From: Sean Paul
>
> The shim functions return error codes, but they are discarded in
> intel_hdcp.c. This patch plumbs the return codes through so they are
> properly handled.
>
> Acked-by: Jani Nikula
> Reviewed-by: Rodrigo Vivi
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
>
> ---
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased
> Changes in v7:
> -None
> Changes in v8:
> -None
>
> .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++-
> drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> 3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 7bcd90384a46d..a14b86a07e545 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -494,6 +494,7 @@ static void intel_panel_info(struct seq_file *m, static
> void intel_hdcp_info(struct seq_file *m,
> struct intel_connector *intel_connector) {
> + int ret;
> bool hdcp_cap, hdcp2_cap;
>
> if (!intel_connector->hdcp.shim) {
> @@ -501,8 +502,12 @@ static void intel_hdcp_info(struct seq_file *m,
> goto out;
> }
>
> - hdcp_cap = intel_hdcp_capable(intel_connector);
> - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> + ret = intel_hdcp_capable(intel_connector, _cap);
> + if (ret)
> + hdcp_cap = false;
> + ret = intel_hdcp2_capable(intel_connector, _cap);
> + if (ret)
> + hdcp2_cap = false;
>
> if (hdcp_cap)
> seq_puts(m, "HDCP1.4 ");
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0a20bc41be55d..61a862ae1f286 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct
> intel_digital_port *dig_port, }
>
> /* Is HDCP1.4 capable on Platform and Sink */ -bool
> intel_hdcp_capable(struct intel_connector *connector)
> +int intel_hdcp_capable(struct intel_connector *connector, bool
> +*capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> - bool capable = false;
> u8 bksv[5];
>
> + *capable = false;
> +
> if (!shim)
> - return capable;
> + return 0;
>
> - if (shim->hdcp_capable) {
> - shim->hdcp_capable(dig_port, );
> - } else {
> - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> - capable = true;
> - }
> + if (shim->hdcp_capable)
> + return shim->hdcp_capable(dig_port, capable);
> +
> + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> + *capable = true;
>
> - return capable;
> + return 0;
> }
>
> /* Is HDCP2.2 capable on Platform and Sink */ -bool
> intel_hdcp2_capable(struct intel_connector *connector)
> +int intel_hdcp2_capable(struct intel_connector *connector, bool
> +*capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_hdcp *hdcp = >hdcp;
> - bool capable = false;
> +
> + *capable = false;
>
> /* I915 support for HDCP2.2 */
> if (!hdcp->hdcp2_supported)
> - return false;
> + return 0;
>
> /* MEI interface is solid */
> mutex_lock(_priv->display.hdcp.comp_mutex);
> if (!dev_priv->display.hdcp.comp_added || !dev_priv-
> >display.hdcp.master) {
> mutex_unlock(_priv->display.hdcp.comp_mutex);
> - return false;
> +
RE: [PATCH v8 04/10] drm/hdcp: Expand HDCP helper library for enable/disable/check
> -Original Message-
> From: Mark Yacoub
> Sent: Saturday, April 1, 2023 3:42 AM
> To: David Airlie ; Daniel Vetter
> Cc: seanp...@chromium.org; Kandpal, Suraj ;
> diand...@chromium.org; dri-devel@lists.freedesktop.org;
> freedr...@lists.freedesktop.org; intel-...@lists.freedesktop.org; Nikula, Jani
> ; Mark Yacoub ; linux-
> ker...@vger.kernel.org
> Subject: [PATCH v8 04/10] drm/hdcp: Expand HDCP helper library for
> enable/disable/check
>
> From: Sean Paul
>
> Expand upon the HDCP helper library to manage HDCP enable, disable, and
> check.
>
> Previous to this patch, the majority of the state management and sink
> interaction is tucked inside the Intel driver with the understanding
> that once a new platform supported HDCP we could make good decisions
> about what should be centralized. With the addition of HDCP support
> for Qualcomm, it's time to migrate the protocol-specific bits of HDCP
> authentication, key exchange, and link checks to the HDCP helper.
>
> In terms of functionality, this migration is 1:1 with the Intel driver,
> however things are laid out a bit differently than with intel_hdcp.c,
> which is why this is a separate patch from the i915 transition to the
> helper. On i915, the shim vtable is used to account for HDMI vs. DP
> vs. DP-MST differences whereas the helper library uses a LUT to
> account for the register offsets and a remote read function to route
> the messages. On i915, storing the sink information in the source is
> done inline whereas now we use the new drm_hdcp_helper_funcs vtable
> to store and fetch information to/from source hw. Finally, instead of
> calling enable/disable directly from the driver, we'll leave that
> decision to the helper and by calling drm_hdcp_helper_atomic_commit()
> from the driver. All told, this will centralize the protocol and state
> handling in the helper, ensuring we collect all of our bugs^Wlogic
> in one place.
>
> Acked-by: Jani Nikula
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
>
> ---
> Changes in v2:
> -Fixed set-but-unused variable identified by 0-day
> Changes in v3:
> -Fixed uninitialized variable warning identified by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Fixed typo in function descriptions
> -Rebased: Moved the new code between drm_hdcp.h and
> drm_hdcp_helper.c/h
> -Add missing headers. Reported-by: kernel test robot
> Changes in v7:
> - Add a |driver_data| field to some functions in drm_hdcp_helper_funcs
> that are called by the driver so drivers can pass anything they such
> as bridges
> - Isolate all non-common code between HDMI and DP into separate
> functions instead of manually checking for datra->aux
> Changes in v8 (suraj):
> -Try hdcp 1.x if hdcp2_capable returns an error instead of goto out.
> -set the enabled type to either 1 or 0 depending on the prop as hdcp2
> can support either.
>
> drivers/gpu/drm/display/drm_hdcp_helper.c | 1215
> +
> include/drm/display/drm_hdcp.h| 287 +
> include/drm/display/drm_hdcp_helper.h | 51 +-
> 3 files changed, 1552 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdcp_helper.c
> b/drivers/gpu/drm/display/drm_hdcp_helper.c
> index 3ee1a6ae26c53..3bc0308cc95d8 100644
> --- a/drivers/gpu/drm/display/drm_hdcp_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdcp_helper.c
> @@ -6,13 +6,18 @@
> * Ramalingam C
> */
>
> +#include
> #include
> #include
> #include
> +#include
> +#include
> #include
> #include
> #include
> +#include
>
> +#include
> #include
> #include
> #include
> @@ -507,3 +512,1213 @@ bool drm_hdcp_has_changed(struct
> drm_connector *connector,
> return old_hdcp != new_hdcp;
> }
> EXPORT_SYMBOL(drm_hdcp_has_changed);
> +
> +struct drm_hdcp_hdcp1_receiver_reg_lut {
> + unsigned int bksv;
> + unsigned int ri;
> + unsigned int aksv;
> + unsigned int an;
> + unsigned int ainfo;
> + unsigned int v[5];
> + unsigned int bcaps;
> + unsigned int bcaps_mask_repeater_present;
> + unsigned int bstatus;
> +};
> +
> +static const struct drm_hdcp_hdcp1_receiver_reg_lut
> drm_hdcp_hdcp1_ddc_lut = {
> + .bksv = DRM_HDCP_DDC_BKSV,
> + .ri = DRM_HDCP_DDC_RI_PRIME,
> + .aksv = DRM_HDCP_DDC_AKSV,
> + .an = DRM_HDCP_DDC_AN,
> + .ainfo = DRM_HDCP_DDC_AINFO,
> + .v = { DRM_HDCP_DDC_V_PRIME(0), DRM_HDCP_DDC_V_PRIME(1),
> +DRM_HDCP_DDC_V_PRIME(2), DRM_HDCP_DDC_V_PRIME(3),
> +DRM_HDCP_DDC_V_PRIME(4) },
> + .bcaps = DRM_HDCP_DDC_BCAPS,
> +
RE: [PATCH 0/7] Enable YCbCr420 format for VDSC
> -Original Message- > From: dri-devel On Behalf Of Jani > Nikula > Sent: Wednesday, March 8, 2023 5:00 PM > To: Kandpal, Suraj ; dri- > de...@lists.freedesktop.org; intel-...@lists.freedesktop.org > Cc: Dmitry Baryshkov ; Nautiyal, Ankit K > ; Shankar, Uma ; > Kandpal, Suraj > Subject: Re: [PATCH 0/7] Enable YCbCr420 format for VDSC > > On Wed, 22 Feb 2023, Suraj Kandpal wrote: > > This patch series aims to enable the YCbCr420 format for DSC. Changes > > are mostly compute params related for hdmi,dp and dsi along with the > > addition of new rc_tables for native_420 and corresponding changes to > > macros used to fetch them. > > There have been discussions prior to this series in which some patches > > have gotten rb and can be found in the below link > > https://patchwork.freedesktop.org/series/113729 > > I think it would be useful to get [1] from Dmitry merged to drm-misc-next > first, have that in drm-next, and again backmerged to drm-intel-next before > this. At least patches 1-5. > > There's not much point in all drivers duplicating the parameters, and we > need to move towards common code. Dmitry has been helpful in > contributing this to us. > > BR, > Jani. > > Hi Jani, Maarten has acked the patch series to be merged through drm-intel and in the meantime I will work with Dmitry to pull the common code to avoid duplication Regards, Suraj Kandpal > [1] https://patchwork.freedesktop.org/series/114473/ > > > > > Ankit Nautiyal (2): > > drm/dp_helper: Add helper to check DSC support with given o/p format > > drm/i915/dp: Check if DSC supports the given output_format > > > > Suraj Kandpal (4): > > drm/i915: Adding the new registers for DSC > > drm/i915: Enable YCbCr420 for VDSC > > drm/i915/display: Fill in native_420 field > > drm/i915/vdsc: Check slice design requirement > > > > Swati Sharma (1): > > drm/i915/dsc: Add debugfs entry to validate DSC output formats > > > > drivers/gpu/drm/i915/display/icl_dsi.c| 2 - > > .../drm/i915/display/intel_crtc_state_dump.c | 4 +- > > .../drm/i915/display/intel_crtc_state_dump.h | 2 + > > .../drm/i915/display/intel_display_debugfs.c | 78 > > .../drm/i915/display/intel_display_types.h| 1 + > > drivers/gpu/drm/i915/display/intel_dp.c | 39 +++- > > .../gpu/drm/i915/display/intel_qp_tables.c| 187 -- > > .../gpu/drm/i915/display/intel_qp_tables.h| 4 +- > > drivers/gpu/drm/i915/display/intel_vdsc.c | 108 +- > > drivers/gpu/drm/i915/i915_reg.h | 28 +++ > > include/drm/display/drm_dp_helper.h | 13 ++ > > 11 files changed, 442 insertions(+), 24 deletions(-) > > -- > Jani Nikula, Intel Open Source Graphics Center
RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
> -Original Message- > From: Mark Yacoub > Sent: Saturday, March 25, 2023 12:57 AM > To: Kandpal, Suraj > Cc: quic_khs...@quicinc.com; linux-arm-...@vger.kernel.org; dri- > de...@lists.freedesktop.org; freedr...@lists.freedesktop.org; > devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; intel- > g...@lists.freedesktop.org; quic_sbill...@quicinc.com; > konrad.dyb...@somainline.org; Souza, Jose ; > bjorn.anders...@linaro.org; krzysztof.kozlowski...@linaro.org; > hbh...@gmail.com; Vasut, Marek ; Dixit, Ashutosh > ; s...@poorly.run; abhin...@codeaurora.org; > javi...@redhat.com; Murthy, Arun R ; Lisovskiy, > Stanislav ; agr...@kernel.org; > quic_jessz...@quicinc.com; Nautiyal, Ankit K ; > Nikula, Jani ; De Marchi, Lucas > ; quic_abhin...@quicinc.com; > swb...@chromium.org; robh...@kernel.org; > christophe.jail...@wanadoo.fr; max...@cerno.tech; Vivi, Rodrigo > ; johan+lin...@kernel.org; > tvrtko.ursu...@linux.intel.com; anders...@kernel.org; > diand...@chromium.org; Sharma, Swati2 ; > Navare, Manasi D ; tzimmerm...@suse.de; > Modem, Bhanuprakash ; > dmitry.barysh...@linaro.org; seanp...@chromium.org > Subject: Re: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return > codes > > On Thu, Mar 23, 2023 at 3:18 AM Kandpal, Suraj > wrote: > > > > > > > > > -Original Message- > > > From: Kandpal, Suraj > > > Sent: Friday, March 10, 2023 1:55 PM > > > To: Mark Yacoub ; > quic_khs...@quicinc.com; > > > linux-arm-...@vger.kernel.org; dri-devel@lists.freedesktop.org; > > > freedr...@lists.freedesktop.org; devicet...@vger.kernel.org; linux- > > > ker...@vger.kernel.org; intel-...@lists.freedesktop.org > > > Cc: quic_sbill...@quicinc.com; konrad.dyb...@somainline.org; Souza, > > > Jose ; bjorn.anders...@linaro.org; > > > krzysztof.kozlowski...@linaro.org; hbh...@gmail.com; Vasut, Marek > > > ; Dixit, Ashutosh ; > > > s...@poorly.run; abhin...@codeaurora.org; javi...@redhat.com; > > > Murthy, Arun R ; Lisovskiy, Stanislav > > > ; agr...@kernel.org; > > > quic_jessz...@quicinc.com; Nautiyal, Ankit K > > > ; Nikula, Jani ; > > > De Marchi, Lucas ; > > > quic_abhin...@quicinc.com; swb...@chromium.org; > robh...@kernel.org; > > > christophe.jail...@wanadoo.fr; max...@cerno.tech; Vivi, Rodrigo > > > ; johan+lin...@kernel.org; > > > tvrtko.ursu...@linux.intel.com; anders...@kernel.org; > > > diand...@chromium.org; Sharma, Swati2 ; > > > Navare, Manasi D ; > tzimmerm...@suse.de; > > > Modem, Bhanuprakash ; > > > dmitry.barysh...@linaro.org; seanp...@chromium.org > > > Subject: RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable > > > return codes > > > > > > > Subject: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable > > > > return codes > > > > > > > > From: Sean Paul > > > > > > > > The shim functions return error codes, but they are discarded in > > > > intel_hdcp.c. This patch plumbs the return codes through so they > > > > are properly handled. > > > > > > > > Acked-by: Jani Nikula > > > > Reviewed-by: Rodrigo Vivi > > > > Signed-off-by: Sean Paul > > > > Signed-off-by: Mark Yacoub > > > > Link: > > > > > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456 > > > > -7- > > > > s...@poorly.run #v1 > > > > Link: > > > > > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439- > > > > 7- > > > > s...@poorly.run #v2 > > > > Link: > > > > > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916 > > > > -7- > > > > s...@poorly.run #v3 > > > > Link: > > > > > > > > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845 > > > - > > > > 7-s...@poorly.run #v4 > > > > Link: > > > > > > > > https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308 > > > - > > > > 7-s...@poorly.run #v5 > > > > > > > > Changes in v2: > > > > -None > > > > Changes in v3: > > > > -None > > > > Changes in v4: > > > > -None > > > > Changes in v5: > > > > -None > > > > Changes in v6: > > > > -Rebased > > > > > > > > --- > > > > .../drm/i915/display/intel_display_debugfs.c | 9 +++- >
RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
> -Original Message-
> From: Kandpal, Suraj
> Sent: Friday, March 10, 2023 1:55 PM
> To: Mark Yacoub ; quic_khs...@quicinc.com;
> linux-arm-...@vger.kernel.org; dri-devel@lists.freedesktop.org;
> freedr...@lists.freedesktop.org; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org; intel-...@lists.freedesktop.org
> Cc: quic_sbill...@quicinc.com; konrad.dyb...@somainline.org; Souza, Jose
> ; bjorn.anders...@linaro.org;
> krzysztof.kozlowski...@linaro.org; hbh...@gmail.com; Vasut, Marek
> ; Dixit, Ashutosh ;
> s...@poorly.run; abhin...@codeaurora.org; javi...@redhat.com; Murthy,
> Arun R ; Lisovskiy, Stanislav
> ; agr...@kernel.org;
> quic_jessz...@quicinc.com; Nautiyal, Ankit K ;
> Nikula, Jani ; De Marchi, Lucas
> ; quic_abhin...@quicinc.com;
> swb...@chromium.org; robh...@kernel.org;
> christophe.jail...@wanadoo.fr; max...@cerno.tech; Vivi, Rodrigo
> ; johan+lin...@kernel.org;
> tvrtko.ursu...@linux.intel.com; anders...@kernel.org;
> diand...@chromium.org; Sharma, Swati2 ;
> Navare, Manasi D ; tzimmerm...@suse.de;
> Modem, Bhanuprakash ;
> dmitry.barysh...@linaro.org; seanp...@chromium.org
> Subject: RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return
> codes
>
> > Subject: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return
> > codes
> >
> > From: Sean Paul
> >
> > The shim functions return error codes, but they are discarded in
> > intel_hdcp.c. This patch plumbs the return codes through so they are
> > properly handled.
> >
> > Acked-by: Jani Nikula
> > Reviewed-by: Rodrigo Vivi
> > Signed-off-by: Sean Paul
> > Signed-off-by: Mark Yacoub
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-7-
> > s...@poorly.run #v1
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-7-
> > s...@poorly.run #v2
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-7-
> > s...@poorly.run #v3
> > Link:
> >
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> > 7-s...@poorly.run #v4
> > Link:
> >
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> > 7-s...@poorly.run #v5
> >
> > Changes in v2:
> > -None
> > Changes in v3:
> > -None
> > Changes in v4:
> > -None
> > Changes in v5:
> > -None
> > Changes in v6:
> > -Rebased
> >
> > ---
> > .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> > drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++-
> > drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> > 3 files changed, 37 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 7c7253a2541c..13a4153bb76e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -492,6 +492,7 @@ static void intel_panel_info(struct seq_file *m,
> > static void intel_hdcp_info(struct seq_file *m,
> > struct intel_connector *intel_connector) {
> > + int ret;
> > bool hdcp_cap, hdcp2_cap;
> >
> > if (!intel_connector->hdcp.shim) {
> > @@ -499,8 +500,12 @@ static void intel_hdcp_info(struct seq_file *m,
> > goto out;
> > }
> >
> > - hdcp_cap = intel_hdcp_capable(intel_connector);
> > - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> > + ret = intel_hdcp_capable(intel_connector, _cap);
> > + if (ret)
> > + hdcp_cap = false;
> > + ret = intel_hdcp2_capable(intel_connector, _cap);
> > + if (ret)
> > + hdcp2_cap = false;
> >
>
> This does not seem to be adding value here as this error which you referred
> to as being ignored is used both in case of hdmi and dp is being to determine
> if hdcp_cap or hdcp2 cap is true or false which you basically reiterate here
> too
> check the intel_dp_hdcp2_capable and intel_hdmi_hdcp2_capable .
> this change in itself can be removed.
>
> Regards,
> Suraj Kandpal
>
> > if (hdcp_cap)
> > seq_puts(m, "HDCP1.4 ");
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 0a20bc41be55..61a862ae1f28 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -177,50 +177,49 @@ int intel_hdcp_read_vali
RE: [Intel-gfx] [PATCH v6 07/10] drm/i915/hdcp: Use HDCP helpers for i915
>
> From: Sean Paul
>
> Now that all of the HDCP 1.x logic has been migrated to the central HDCP
> helpers, use it in the i915 driver.
>
> The majority of the driver code for HDCP 1.x will live in intel_hdcp.c,
> however there are a few helper hooks which are connector-specific and
> need to be partially or fully implemented in the intel_dp_hdcp.c or
> intel_hdmi.c.
>
> We'll leave most of the HDCP 2.x code alone since we don't have another
> implementation of HDCP 2.x to use as reference for what should and
> should not live in the drm helpers. The helper will call the overly
> general enable/disable/is_capable HDCP 2.x callbacks and leave the
> interesting stuff for the driver. Once we have another HDCP 2.x
> implementation, we should do a similar migration.
>
> Acked-by: Jani Nikula
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-8-
> s...@poorly.run #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-8-
> s...@poorly.run #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-8-
> s...@poorly.run #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> 8-s...@poorly.run #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> 8-s...@poorly.run #v5
>
> Changes in v2:
> -Fix mst helper function pointer reported by 0-day
> Changes in v3:
> -Add forward declaration for drm_atomic_state in intel_hdcp.h identified
> by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased.
>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 32 +-
> .../drm/i915/display/intel_display_debugfs.c | 6 +-
> .../drm/i915/display/intel_display_types.h| 60 +-
> drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 348 +++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 952 +++---
> drivers/gpu/drm/i915/display/intel_hdcp.h | 31 +-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 270 ++---
> 8 files changed, 445 insertions(+), 1270 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 69ecf2a3d6c6..a4397f066a3e 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -28,6 +28,7 @@
> #include
>
> #include
> +#include
> #include
>
> #include "i915_drv.h"
> @@ -2909,6 +2910,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
>const struct intel_crtc_state *crtc_state,
>const struct drm_connector_state *conn_state)
> {
> + struct intel_connector *connector =
> + to_intel_connector(conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
> drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
>
> if (!intel_crtc_is_bigjoiner_slave(crtc_state))
> @@ -2925,12 +2930,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
> else
> intel_enable_ddi_dp(state, encoder, crtc_state,
> conn_state);
>
> - /* Enable hdcp if it's desired */
> - if (conn_state->content_protection ==
> - DRM_MODE_CONTENT_PROTECTION_DESIRED)
> - intel_hdcp_enable(to_intel_connector(conn_state-
> >connector),
> - crtc_state,
> - (u8)conn_state->hdcp_content_type);
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> + >base,
> + _port->hdcp_mutex);
> }
>
> static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> @@ -2976,7 +2979,14 @@ static void intel_disable_ddi(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state
> *old_conn_state)
> {
> - intel_hdcp_disable(to_intel_connector(old_conn_state-
> >connector));
> + struct intel_connector *connector =
> + to_intel_connector(old_conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> + >base,
> + _port->hdcp_mutex);
>
> if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
> intel_disable_ddi_hdmi(state, encoder, old_crtc_state,
> @@ -3004,13 +3014,19 @@ void intel_ddi_update_pipe(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *crtc_state,
>
RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
> Subject: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
>
> From: Sean Paul
>
> The shim functions return error codes, but they are discarded in
> intel_hdcp.c. This patch plumbs the return codes through so they are
> properly handled.
>
> Acked-by: Jani Nikula
> Reviewed-by: Rodrigo Vivi
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-7-
> s...@poorly.run #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-7-
> s...@poorly.run #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-7-
> s...@poorly.run #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> 7-s...@poorly.run #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> 7-s...@poorly.run #v5
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased
>
> ---
> .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++-
> drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> 3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 7c7253a2541c..13a4153bb76e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -492,6 +492,7 @@ static void intel_panel_info(struct seq_file *m, static
> void intel_hdcp_info(struct seq_file *m,
> struct intel_connector *intel_connector) {
> + int ret;
> bool hdcp_cap, hdcp2_cap;
>
> if (!intel_connector->hdcp.shim) {
> @@ -499,8 +500,12 @@ static void intel_hdcp_info(struct seq_file *m,
> goto out;
> }
>
> - hdcp_cap = intel_hdcp_capable(intel_connector);
> - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> + ret = intel_hdcp_capable(intel_connector, _cap);
> + if (ret)
> + hdcp_cap = false;
> + ret = intel_hdcp2_capable(intel_connector, _cap);
> + if (ret)
> + hdcp2_cap = false;
>
This does not seem to be adding value here as this error which you referred to
as being ignored
is used both in case of hdmi and dp is being to determine if hdcp_cap or hdcp2
cap is true or false
which you basically reiterate here too
check the intel_dp_hdcp2_capable and intel_hdmi_hdcp2_capable .
this change in itself can be removed.
Regards,
Suraj Kandpal
> if (hdcp_cap)
> seq_puts(m, "HDCP1.4 ");
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0a20bc41be55..61a862ae1f28 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct
> intel_digital_port *dig_port, }
>
> /* Is HDCP1.4 capable on Platform and Sink */ -bool
> intel_hdcp_capable(struct intel_connector *connector)
> +int intel_hdcp_capable(struct intel_connector *connector, bool
> +*capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> - bool capable = false;
> u8 bksv[5];
>
> + *capable = false;
> +
> if (!shim)
> - return capable;
> + return 0;
>
> - if (shim->hdcp_capable) {
> - shim->hdcp_capable(dig_port, );
> - } else {
> - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> - capable = true;
> - }
> + if (shim->hdcp_capable)
> + return shim->hdcp_capable(dig_port, capable);
> +
> + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> + *capable = true;
>
> - return capable;
> + return 0;
> }
>
> /* Is HDCP2.2 capable on Platform and Sink */ -bool
> intel_hdcp2_capable(struct intel_connector *connector)
> +int intel_hdcp2_capable(struct intel_connector *connector, bool
> +*capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_hdcp *hdcp = >hdcp;
> - bool capable = false;
> +
> + *capable = false;
>
> /* I915 support for HDCP2.2 */
> if (!hdcp->hdcp2_supported)
> - return false;
> + return 0;
>
> /* MEI interface is solid */
> mutex_lock(_priv->display.hdcp.comp_mutex);
> if (!dev_priv->display.hdcp.comp_added || !dev_priv-
> >display.hdcp.master) {
> mutex_unlock(_priv->display.hdcp.comp_mutex);
> - return false;
> + return 0;
> }
>
RE: [Intel-gfx] [PATCH v6 02/10] drm/hdcp: Avoid changing crtc state in hdcp atomic check
> -Original Message-
> From: Intel-gfx On Behalf Of Mark
> Yacoub
> Sent: Thursday, January 19, 2023 1:00 AM
> To: quic_khs...@quicinc.com; linux-arm-...@vger.kernel.org; dri-
> de...@lists.freedesktop.org; freedr...@lists.freedesktop.org;
> devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; intel-
> g...@lists.freedesktop.org
> Cc: quic_sbill...@quicinc.com; konrad.dyb...@somainline.org;
> bjorn.anders...@linaro.org; krzysztof.kozlowski...@linaro.org;
> airl...@gmail.com; hbh...@gmail.com; Vasut, Marek ;
> abhin...@codeaurora.org; javi...@redhat.com; agr...@kernel.org;
> quic_jessz...@quicinc.com; dan...@ffwll.ch; Nikula, Jani
> ; De Marchi, Lucas ;
> quic_abhin...@quicinc.com; swb...@chromium.org; robh...@kernel.org;
> christophe.jail...@wanadoo.fr; max...@cerno.tech; Vivi, Rodrigo
> ; johan+lin...@kernel.org;
> markyac...@chromium.org; anders...@kernel.org;
> diand...@chromium.org; tzimmerm...@suse.de;
> dmitry.barysh...@linaro.org; seanp...@chromium.org
> Subject: [Intel-gfx] [PATCH v6 02/10] drm/hdcp: Avoid changing crtc state in
> hdcp atomic check
>
> From: Sean Paul
>
> Instead of forcing a modeset in the hdcp atomic check, simply return true if
> the content protection value is changing and let the driver decide whether a
> modeset is required or not.
>
> Acked-by: Jani Nikula
> Reviewed-by: Rodrigo Vivi
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-3-
> s...@poorly.run #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-3-
> s...@poorly.run #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-3-
> s...@poorly.run #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> 3-s...@poorly.run #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> 3-s...@poorly.run #v5
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in V6:
> -Rebase: modifications in drm_hdcp_helper.c instead of drm_hdcp.c
>
> ---
> drivers/gpu/drm/display/drm_hdcp_helper.c | 33 +++--
> drivers/gpu/drm/i915/display/intel_atomic.c | 6 ++--
> include/drm/display/drm_hdcp_helper.h | 2 +-
> 3 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdcp_helper.c
> b/drivers/gpu/drm/display/drm_hdcp_helper.c
> index 7d910523b05f..a3896b0904b5 100644
> --- a/drivers/gpu/drm/display/drm_hdcp_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdcp_helper.c
> @@ -428,11 +428,14 @@
> EXPORT_SYMBOL(drm_hdcp_update_content_protection);
> * @connector: drm_connector on which content protection state needs an
> update
> *
> * This function can be used by display drivers to perform an atomic check
> on the
> - * hdcp state elements. If hdcp state has changed, this function will set
> - * mode_changed on the crtc driving the connector so it can update its
> hardware
> - * to match the hdcp state.
> + * hdcp state elements. If hdcp state has changed in a manner which
> + requires the
> + * driver to enable or disable content protection, this function will
> + return
> + * true.
> + *
> + * Returns:
> + * true if the driver must enable/disable hdcp, false otherwise
> */
> -void drm_hdcp_atomic_check(struct drm_connector *connector,
> +bool drm_hdcp_atomic_check(struct drm_connector *connector,
> struct drm_atomic_state *state)
> {
> struct drm_connector_state *new_conn_state, *old_conn_state;
> @@ -450,10 +453,12 @@ void drm_hdcp_atomic_check(struct
> drm_connector *connector,
>* If the connector is being disabled with CP enabled, mark it
>* desired so it's re-enabled when the connector is brought
> back
>*/
> - if (old_hdcp ==
> DRM_MODE_CONTENT_PROTECTION_ENABLED)
> + if (old_hdcp ==
> DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> new_conn_state->content_protection =
>
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> - return;
> + return true;
> + }
> + return false;
> }
>
> new_crtc_state =
> @@ -465,9 +470,19 @@ void drm_hdcp_atomic_check(struct
> drm_connector *connector,
> */
> if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> - new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> + new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> {
> new_conn_state->content_protection =
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return true;
> + }
> +
> + /*
> + * Coming back from disable or changing CRTC with DESIRED state
> requires
> + * that the driver try CP enable.
> + */
Hi ,
We can have a clearer comment which says something like "Coming
RE: [Intel-gfx] [PATCH v6 01/10] drm/hdcp: Add drm_hdcp_atomic_check()
>
> From: Sean Paul
>
> This patch moves the hdcp atomic check from i915 to drm_hdcp so other
> drivers can use it. No functional changes, just cleaned up some of the code
> when moving it over.
>
> Acked-by: Jani Nikula
> Acked-by: Jani Nikula
> Reviewed-by: Rodrigo Vivi
> Reviewed-by: Abhinav Kumar
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-2-
> s...@poorly.run #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-2-
> s...@poorly.run #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-2-
> s...@poorly.run #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> 2-s...@poorly.run #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> 2-s...@poorly.run #v5
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in V6:
> -Rebase: move helper from drm_hdcp.c to drm_hdcp_helper.c
>
> ---
> drivers/gpu/drm/display/drm_hdcp_helper.c | 69
> +
> drivers/gpu/drm/i915/display/intel_atomic.c | 4 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 47 --
> drivers/gpu/drm/i915/display/intel_hdcp.h | 3 -
> include/drm/display/drm_hdcp_helper.h | 3 +
> 5 files changed, 74 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdcp_helper.c
> b/drivers/gpu/drm/display/drm_hdcp_helper.c
> index e78999c72bd7..7d910523b05f 100644
> --- a/drivers/gpu/drm/display/drm_hdcp_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdcp_helper.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> static inline void drm_hdcp_print_ksv(const u8 *ksv) { @@ -419,3 +420,71
> @@ void drm_hdcp_update_content_protection(struct drm_connector
> *connector,
>dev-
> >mode_config.content_protection_property);
> }
> EXPORT_SYMBOL(drm_hdcp_update_content_protection);
> +
> +/**
> + * drm_hdcp_atomic_check - Helper for drivers to call during
> +connector->atomic_check
> + *
> + * @state: pointer to the atomic state being checked
> + * @connector: drm_connector on which content protection state needs an
> +update
> + *
> + * This function can be used by display drivers to perform an atomic
> +check on the
> + * hdcp state elements. If hdcp state has changed, this function will
> +set
> + * mode_changed on the crtc driving the connector so it can update its
> +hardware
> + * to match the hdcp state.
> + */
> +void drm_hdcp_atomic_check(struct drm_connector *connector,
> +struct drm_atomic_state *state)
> +{
> + struct drm_connector_state *new_conn_state, *old_conn_state;
> + struct drm_crtc_state *new_crtc_state;
> + u64 old_hdcp, new_hdcp;
> +
> + old_conn_state = drm_atomic_get_old_connector_state(state,
> connector);
> + old_hdcp = old_conn_state->content_protection;
> +
> + new_conn_state = drm_atomic_get_new_connector_state(state,
> connector);
> + new_hdcp = new_conn_state->content_protection;
> +
> + if (!new_conn_state->crtc) {
> + /*
> + * If the connector is being disabled with CP enabled, mark it
> + * desired so it's re-enabled when the connector is brought
> back
> + */
> + if (old_hdcp ==
> DRM_MODE_CONTENT_PROTECTION_ENABLED)
> + new_conn_state->content_protection =
> +
> DRM_MODE_CONTENT_PROTECTION_DESIRED;
> + return;
> + }
> +
> + new_crtc_state =
> + drm_atomic_get_new_crtc_state(state, new_conn_state-
> >crtc);
> + /*
> + * Fix the HDCP uapi content protection state in case of modeset.
> + * FIXME: As per HDCP content protection property uapi doc, an
> uevent()
> + * need to be sent if there is transition from ENABLED->DESIRED.
> + */
Hi Mark,
Is the above comment needed here as drm_hdcp_update_content_protection is
used to change property which sends a uevent making the above comment misleading
Regards,
Suraj Kandpal
> + if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> + new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
> + new_conn_state->content_protection =
> + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> +
> + /*
> + * Nothing to do if content type is unchanged and one of:
> + * - state didn't change
> + * - HDCP was activated since the last commit
> + * - attempting to set to desired while already enabled
> + */
> + if (old_hdcp == new_hdcp ||
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> + new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> + new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED))
RE: [Intel-gfx] [PATCH 0/3] i915 private writeback framework
Hi Daniel, > > A patch series was floated in the drm mailing list which aimed to > > change the drm_connector and drm_encoder fields to pointer in the > > drm_connector_writeback structure, this received a huge pushback from > > the community but since i915 expects each connector present in the > > drm_device list to be a intel_connector but drm_writeback framework. > > [1] > > https://patchwork.kernel.org/project/dri-devel/patch/20220202081702.22 > > 119-1-suraj.kand...@intel.com/ [2] > > https://patchwork.kernel.org/project/dri-devel/patch/20220202085429.22 > > 261-6-suraj.kand...@intel.com/ This forces us to use a drm_connector > > which is not embedded in intel_connector the current drm_writeback > > framework becomes very unfeasible to us as it would mean a lot of > > checks at a lot of places to take into account the above issue.Since > > no one had an issue with encoder field being changed into a pointer it > > was decided to break the connector and encoder pointer changes into > > two different series.The encoder field changes is currently being > > worked upon by Abhinav Kumar > > [3]https://patchwork.kernel.org/project/dri-devel/list/?series=633565 > > In the meantime for i915 to start using the writeback functionality we > > came up with a interim solution to own writeback pipeline bypassing > > one provided by drm which is what these patches do. > > Note: these are temp patches till we figure out how we can either > > change drm core writeback to work with our intel_connector structure > > or find a different solution which allows us to work with the current > > I'm assuming this is just fyi to keep development moving and not being > planned for merging? Yes we do plan to get it merged as a proper implementation that uses drm-core will require significant time and to unblock the writeback functionality these interim series of patches have been floated. Regards Suraj Kandpal
RE: [RFC PATCH 1/3] drm/i915: Creating writeback pipeline to bypass drm_writeback framework
++Laurent ,Dmitry, and Abhinav
> Changes to create a i915 private pipeline to enable the WD transcoder
> without relying on the current drm_writeback framework.
>
> Signed-off-by: Suraj Kandpal
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> .../drm/i915/display/intel_display_types.h| 4 +
> .../gpu/drm/i915/display/intel_wb_connector.c | 296
> ++ .../gpu/drm/i915/display/intel_wb_connector.h |
> 99 ++
> drivers/gpu/drm/i915/i915_drv.h | 3 +
> 5 files changed, 403 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 1a771ee5b1d0..087bd9d1b397 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -286,6 +286,7 @@ i915-y += \
> display/intel_tv.o \
> display/intel_vdsc.o \
> display/intel_vrr.o \
> + display/intel_wb_connector.o\
> display/vlv_dsi.o \
> display/vlv_dsi_pll.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index d84e82f3eab9..7a96ecba73c0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -52,6 +52,7 @@
> #include "intel_display_power.h"
> #include "intel_dpll_mgr.h"
> #include "intel_pm_types.h"
> +#include "intel_wb_connector.h"
>
> struct drm_printer;
> struct __intel_global_objs_state;
> @@ -537,11 +538,14 @@ struct intel_connector {
> struct work_struct modeset_retry_work;
>
> struct intel_hdcp hdcp;
> +
> + struct intel_writeback_connector wb_conn;
> };
>
> struct intel_digital_connector_state {
> struct drm_connector_state base;
>
> + struct intel_writeback_job *job;
> enum hdmi_force_audio force_audio;
> int broadcast_rgb;
> };
> diff --git a/drivers/gpu/drm/i915/display/intel_wb_connector.c
> b/drivers/gpu/drm/i915/display/intel_wb_connector.c
> new file mode 100644
> index ..65f4abef53d0
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_wb_connector.c
> @@ -0,0 +1,296 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2022 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> +"Software"),
> + * to deal in the Software without restriction, including without
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> +next
> + * paragraph) shall be included in all copies or substantial portions
> +of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT
> +SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES OR
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER
> +DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Suraj Kandpal
> + * Arun Murthy
> + *
> + */
> +
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "i915_drv.h"
> +#include "intel_wb_connector.h"
> +#include "intel_display_types.h"
> +
> +#define fence_to_wb_connector(x) container_of(x->lock, \
> + struct intel_writeback_connector,
> \
> + fence_lock)
> +
> +static const char *intel_writeback_fence_get_driver_name(struct
> +dma_fence *fence) {
> + struct intel_writeback_connector *wb_connector =
> + fence_to_wb_connector(fence);
> +
> + return wb_connector->base->dev->driver->name;
> +}
> +
> +static const char *
> +intel_writeback_fence_get_timeline_name(struct dma_fence *fence) {
> + struct intel_writeback_connector *wb_connector =
> + fence_to_wb_connector(fence);
> +
> + return wb_connector->timeline_name;
> +}
> +
> +static bool intel_writeback_fence_enable_signaling(struct dma_fence
> +*fence) {
> + return true;
> +}
> +
> +static const struct dma_fence_ops intel_writeback_fence_ops = {
> + .get_driver_name = intel_writeback_fence_get_driver_name,
> + .get_timeline_name = intel_writeback_fence_get_timeline_name,
> + .enable_signaling = intel_writeback_fence_enable_signaling,
> +};
> +
> +static int
RE: [RFC PATCH 3/3] drm/i915: Enabling WD Transcoder
++Laurent ,Dmitry, Abhinav and Rob
> Adding support for writeback transcoder to start capturing frames using
> interrupt mechanism
>
> Signed-off-by: Suraj Kandpal
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/display/intel_acpi.c | 1 +
> drivers/gpu/drm/i915/display/intel_display.c | 89 +-
> drivers/gpu/drm/i915/display/intel_display.h | 9 +
> .../drm/i915/display/intel_display_types.h| 13 +
> drivers/gpu/drm/i915/display/intel_dpll.c | 3 +
> drivers/gpu/drm/i915/display/intel_opregion.c | 3 +
> drivers/gpu/drm/i915/display/intel_wd.c | 978 ++
> drivers/gpu/drm/i915/display/intel_wd.h | 82 ++
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_irq.c | 8 +-
> drivers/gpu/drm/i915/i915_pci.c | 7 +-
> drivers/gpu/drm/i915/i915_reg.h | 137 +++
> 13 files changed, 1330 insertions(+), 3 deletions(-) create mode 100644
> drivers/gpu/drm/i915/display/intel_wd.c
> create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 087bd9d1b397..5ee32513a945 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -287,6 +287,7 @@ i915-y += \
> display/intel_vdsc.o \
> display/intel_vrr.o \
> display/intel_wb_connector.o\
> + display/intel_wd.o\
> display/vlv_dsi.o \
> display/vlv_dsi_pll.o
>
> diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
> b/drivers/gpu/drm/i915/display/intel_acpi.c
> index e78430001f07..ae08db164f73 100644
> --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> @@ -247,6 +247,7 @@ static u32 acpi_display_type(struct intel_connector
> *connector)
> case DRM_MODE_CONNECTOR_LVDS:
> case DRM_MODE_CONNECTOR_eDP:
> case DRM_MODE_CONNECTOR_DSI:
> + case DRM_MODE_CONNECTOR_WRITEBACK:
> display_type = ACPI_DISPLAY_TYPE_INTERNAL_DIGITAL;
> break;
> case DRM_MODE_CONNECTOR_Unknown:
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index eb49973621f0..6dedc7921f54 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -111,6 +111,7 @@
> #include "intel_sprite.h"
> #include "intel_tc.h"
> #include "intel_vga.h"
> +#include "intel_wd.h"
> #include "i9xx_plane.h"
> #include "skl_scaler.h"
> #include "skl_universal_plane.h"
> @@ -1544,6 +1545,72 @@ static void
> intel_encoders_update_complete(struct intel_atomic_state *state)
> }
> }
>
> +static void intel_queue_writeback_job(struct intel_atomic_state *state,
> + struct intel_crtc *intel_crtc, struct intel_crtc_state
> *crtc_state) {
> + struct drm_connector_state *new_conn_state;
> + struct drm_connector *connector;
> + struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> + struct intel_wd *intel_wd;
> + struct intel_connector *intel_connector;
> + struct intel_digital_connector_state *intel_conn_state;
> + struct intel_encoder *encoder;
> + int i;
> +
> + for_each_intel_encoder_with_wd(>drm, encoder) {
> + intel_wd = enc_to_intel_wd(encoder);
> +
> + if (intel_wd->wd_crtc != intel_crtc)
> + return;
> +
> + }
> +
> + for_each_new_connector_in_state(>base, connector,
> new_conn_state,
> + i) {
> + intel_conn_state =
> to_intel_digital_connector_state(new_conn_state);
> + if (!intel_conn_state->job)
> + continue;
> + intel_connector = to_intel_connector(connector);
> + intel_writeback_queue_job(_connector->wb_conn,
> new_conn_state);
> + drm_dbg_kms(>drm, "queueing writeback job\n");
> + }
> +}
> +
> +static void intel_find_writeback_connector(struct intel_atomic_state
> *state,
> + struct intel_crtc *intel_crtc, struct intel_crtc_state
> *crtc_state) {
> + struct drm_connector_state *new_conn_state;
> + struct drm_connector *connector;
> + struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> + struct intel_wd *intel_wd;
> + struct intel_encoder *encoder;
> + int i;
> +
> + for_each_intel_encoder_with_wd(>drm, encoder) {
> + intel_wd = enc_to_intel_wd(encoder);
> +
> + if (intel_wd->wd_crtc != intel_crtc)
> + return;
> +
> + }
> +
> + for_each_new_connector_in_state(>base, connector,
> new_conn_state,
> + i) {
> + struct intel_connector *intel_connector;
> +
> + intel_connector = to_intel_connector(connector);
> + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s]: status:
> %s\n",
> +
RE: [RFC PATCH 2/3] drm/i915: Define WD trancoder for i915
++Laurent ,Dmitry, Abhinav and Rob
> -Original Message-
> From: Kandpal, Suraj
> Sent: Thursday, April 21, 2022 10:38 AM
> To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Nikula, Jani ; ville.syrj...@linux.intel.com;
> Murthy, Arun R ; Kandpal, Suraj
>
> Subject: [RFC PATCH 2/3] drm/i915: Define WD trancoder for i915
>
> Adding WD Types, WD transcoder to enum list and WD Transcoder offsets
>
> Signed-off-by: Suraj Kandpal
> ---
> drivers/gpu/drm/i915/display/intel_display.h | 6 ++
> drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/i915_reg.h| 2 ++
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index 8513703086b7..8c93a5de8e07 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -119,6 +119,8 @@ enum transcoder {
> TRANSCODER_DSI_1,
> TRANSCODER_DSI_A = TRANSCODER_DSI_0,/* legacy DSI */
> TRANSCODER_DSI_C = TRANSCODER_DSI_1,/* legacy DSI */
> + TRANSCODER_WD_0,
> + TRANSCODER_WD_1,
>
> I915_MAX_TRANSCODERS
> };
> @@ -140,6 +142,10 @@ static inline const char *transcoder_name(enum
> transcoder transcoder)
> return "DSI A";
> case TRANSCODER_DSI_C:
> return "DSI C";
> + case TRANSCODER_WD_0:
> + return "WD 0";
> + case TRANSCODER_WD_1:
> + return "WD 1";
> default:
> return "";
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 7a96ecba73c0..dcb4ad43cf88 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -79,6 +79,7 @@ enum intel_output_type {
> INTEL_OUTPUT_DSI = 9,
> INTEL_OUTPUT_DDI = 10,
> INTEL_OUTPUT_DP_MST = 11,
> + INTEL_OUTPUT_WD = 12,
> };
>
> enum hdmi_force_audio {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index ddbc7a685a50..6396afd77209
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2023,6 +2023,8 @@
> #define TRANSCODER_EDP_OFFSET 0x6f000
> #define TRANSCODER_DSI0_OFFSET 0x6b000
> #define TRANSCODER_DSI1_OFFSET 0x6b800
> +#define TRANSCODER_WD0_OFFSET0x6e000
> +#define TRANSCODER_WD1_OFFSET0x6e800
>
> #define HTOTAL(trans)_MMIO_TRANS2(trans, _HTOTAL_A)
> #define HBLANK(trans)_MMIO_TRANS2(trans, _HBLANK_A)
> --
> 2.35.1
RE: [RFC PATCH 0/3] i915 writeback private framework
++Laurent ,Dmitry, and Abhinav Hi, Can you have a look at the private implementation i915 is currently going with till we can figure out how to work with drm core . Regards, Suraj Kandpal > A patch series was floated in the drm mailing list which aimed to change the > drm_connector and drm_encoder fields to pointer in the > drm_connector_writeback structure, this received a huge pushback from the > community but since i915 expects each connector present in the drm_device > list to be a intel_connector but drm_writeback framework. > [1] https://patchwork.kernel.org/project/dri- > devel/patch/20220202081702.22119-1-suraj.kand...@intel.com/ > [2] https://patchwork.kernel.org/project/dri- > devel/patch/20220202085429.22261-6-suraj.kand...@intel.com/ > This forces us to use a drm_connector which is not embedded in > intel_connector the current drm_writeback framework becomes very > unfeasible to us as it would mean a lot of checks at a lot of places to take > into > account the above issue.Since no one had an issue with encoder field being > changed into a pointer it was decided to break the connector and encoder > pointer changes into two different series.The encoder field changes is > currently being worked upon by Abhinav Kumar > [3]https://patchwork.kernel.org/project/dri-devel/list/?series=633565 > In the meantime for i915 to start using the writeback functionality we came > up with a interim solution to own writeback pipeline bypassing one provided > by drm which is what these patches do. > Note: these are temp patches till we figure out how we can either change > drm core writeback to work with our intel_connector structure or find a > different solution which allows us to work with the current drm_writeback > framework > > Suraj Kandpal (3): > drm/i915: Creating writeback pipeline to bypass drm_writeback > framework > drm/i915: Define WD trancoder for i915 > drm/i915: Enabling WD Transcoder > > drivers/gpu/drm/i915/Makefile | 2 + > drivers/gpu/drm/i915/display/intel_acpi.c | 1 + > drivers/gpu/drm/i915/display/intel_display.c | 89 +- > drivers/gpu/drm/i915/display/intel_display.h | 15 + > .../drm/i915/display/intel_display_types.h| 18 + > drivers/gpu/drm/i915/display/intel_dpll.c | 3 + > drivers/gpu/drm/i915/display/intel_opregion.c | 3 + > .../gpu/drm/i915/display/intel_wb_connector.c | 296 ++ > .../gpu/drm/i915/display/intel_wb_connector.h | 99 ++ > drivers/gpu/drm/i915/display/intel_wd.c | 978 ++ > drivers/gpu/drm/i915/display/intel_wd.h | 82 ++ > drivers/gpu/drm/i915/i915_drv.h | 5 + > drivers/gpu/drm/i915/i915_irq.c | 8 +- > drivers/gpu/drm/i915/i915_pci.c | 7 +- > drivers/gpu/drm/i915/i915_reg.h | 139 +++ > 15 files changed, 1742 insertions(+), 3 deletions(-) create mode 100644 > drivers/gpu/drm/i915/display/intel_wb_connector.c > create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.h > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c > create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h > > -- > 2.35.1
RE: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi Abhinav, > > Hi, > >>> Hi, > > Hi Abhinav, > >> Hi Laurent > >> > >> Ok sure, I can take this up but I need to understand the proposal > >> a little bit more before proceeding on this. So we will discuss > >> this in another email where we specifically talk about the > >> connector changes. > >> > >> Also, I am willing to take this up once the encoder part is done > >> and merged so that atleast we keep moving on that as MSM WB > >> implementation can proceed with that first. > >> > >> Hi Jani and Suraj > >> > >> Any concerns with splitting up the series into encoder and > >> connector OR re- arranging them? > >> > >> Let me know if you can do this OR I can also split this up > >> keeping Suraj's name in the Co-developed by tag. > > I was actually thinking of floating a new patch series with both > > encoder and connector pointer changes but with a change in > > initialization functions wherein we allocate a connector and > > encoder incase the driver doesn’t have its own this should > > minimize the effect on other drivers already using current drm > > writeback framework and also > allow i915 to create its own connector. > > > I was proposing to split up the encoder and connector because the > comments from Laurent meant we were in agreement about the encoder > but not about the connector. > > I am afraid another attempt with the similar idea is only going to stall the > series again and hence i gave this option. Seems like this patch series currently won't be able to move forward with the connector pointer change so I guess you can split the series to encoder pointer change where we optionally create encoder if required ,keeping my name in co-developed tag so that msm can move forward with its implementation and then we can work to see how the connector issue can be bypassed. Best Regards, Suraj Kandpal > Eventually its upto Laurent and the other maintainers to guide us forward on > this as this series has been stalled for weeks now. > > I thought that Laurent was quite strict about the connector. So I'd > suggest targeting drm_writeback_connector with an optionally > created encoder and the builtin connector > >>> In that case even if we target that i915 will not be able to move > >>> forward with its implementation of writeback as builtin connector > >>> does not work for us right now as explained earlier, optionally > >>> creating encoder and connector will help everyone move forward and > >>> once done > >> we > >>> can actually sit and work on how to side step this issue using > >>> Laurent's suggestion > >> > >> This is up to Laurent & other DRM maintainers to decide whether this > >> approach would be accepted or not. > >> So far unfortunately I have mostly seen the pushback and a strong > >> suggestion to stop treating each drm_connector as i915_connector. > >> I haven't checked the exact details, but IMO adding a branch if the > >> connector's type is DRM_CONNECTOR_VIRTUAL should be doable. > > Bringing in the change where we don’t treat each drm_connector as an > > intel_connector or even adding a branch which checks if drm_connector > > is DRM_CONNECTOR_VIRTUAL and conditionally cast it to intel_connector > > is quite a lot of work for i915. > > That's why I was suggesting if for now we could move forward with > > optionally creating both encoder and connector minimizing affects on > > other drivers as well as allowing us to move forward. > > What do you think, Launrent? > > > >> > > > We can work on Laurent's suggestion but that would first require > > the initial RFC patches and then a rework from both of our drivers > > side to see if they gel well with our current code which will take > > a considerable amount of time. We can for now however float the > > patch series up which minimally affects the current drivers and > > also allows > > i915 and msm to move forward with its writeback implementation off > > course with a proper documentation stating new drivers shouldn't > > try to > make their own connectors and encoders and that drm_writeback will > do that for them. > > Once that's done and merged we can work with Laurent on his > > proposal to improve the drm writeback framework so that this issue > > can be side > stepped entirely in future. > > For now I would like to keep the encoder and connector pointer > > changes > together. > >>> > >>
RE: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi, > > Hi, > > > > Hi Abhinav, > > > > > Hi Laurent > > > > > > > > > > Ok sure, I can take this up but I need to understand the > > > > > proposal a little bit more before proceeding on this. So we will > > > > > discuss this in another email where we specifically talk about the > connector changes. > > > > > > > > > > Also, I am willing to take this up once the encoder part is done > > > > > and merged so that atleast we keep moving on that as MSM WB > > > > > implementation can proceed with that first. > > > > > > > > > > Hi Jani and Suraj > > > > > > > > > > Any concerns with splitting up the series into encoder and > > > > > connector OR re- arranging them? > > > > > > > > > > Let me know if you can do this OR I can also split this up > > > > > keeping Suraj's name in the Co-developed by tag. > > > > I was actually thinking of floating a new patch series with both > > > > encoder and connector pointer changes but with a change in > > > > initialization functions wherein we allocate a connector and > > > > encoder incase the driver doesn’t have its own this should > > > > minimize the effect on other drivers already using current drm > > > > writeback framework and also > > > allow i915 to create its own connector. > > > > > > I thought that Laurent was quite strict about the connector. So I'd > > > suggest targeting drm_writeback_connector with an optionally created > > > encoder and the builtin connector > > In that case even if we target that i915 will not be able to move > > forward with its implementation of writeback as builtin connector does > > not work for us right now as explained earlier, optionally creating > > encoder and connector will help everyone move forward and once done > we > > can actually sit and work on how to side step this issue using > > Laurent's suggestion > > This is up to Laurent & other DRM maintainers to decide whether this > approach would be accepted or not. > So far unfortunately I have mostly seen the pushback and a strong > suggestion to stop treating each drm_connector as i915_connector. > I haven't checked the exact details, but IMO adding a branch if the > connector's type is DRM_CONNECTOR_VIRTUAL should be doable. Bringing in the change where we don’t treat each drm_connector as an intel_connector or even adding a branch which checks if drm_connector is DRM_CONNECTOR_VIRTUAL and conditionally cast it to intel_connector is quite a lot of work for i915. That's why I was suggesting if for now we could move forward with optionally creating both encoder and connector minimizing affects on other drivers as well as allowing us to move forward. What do you think, Launrent? > > > > > > > > We can work on Laurent's suggestion but that would first require > > > > the initial RFC patches and then a rework from both of our drivers > > > > side to see if they gel well with our current code which will take > > > > a considerable amount of time. We can for now however float the > > > > patch series up which minimally affects the current drivers and > > > > also allows > > > > i915 and msm to move forward with its writeback implementation off > > > > course with a proper documentation stating new drivers shouldn't > > > > try to > > > make their own connectors and encoders and that drm_writeback will > > > do that for them. > > > > Once that's done and merged we can work with Laurent on his > > > > proposal to improve the drm writeback framework so that this issue > > > > can be side > > > stepped entirely in future. > > > > For now I would like to keep the encoder and connector pointer > > > > changes > > > together. > > > > > > -- > With best wishes > Dmitry Best Regards, Suraj Kandpal
RE: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi, > > Hi Abhinav, > > > Hi Laurent > > > > > > Ok sure, I can take this up but I need to understand the proposal a > > > little bit more before proceeding on this. So we will discuss this > > > in another email where we specifically talk about the connector changes. > > > > > > Also, I am willing to take this up once the encoder part is done and > > > merged so that atleast we keep moving on that as MSM WB > > > implementation can proceed with that first. > > > > > > Hi Jani and Suraj > > > > > > Any concerns with splitting up the series into encoder and connector > > > OR re- arranging them? > > > > > > Let me know if you can do this OR I can also split this up keeping > > > Suraj's name in the Co-developed by tag. > > I was actually thinking of floating a new patch series with both > > encoder and connector pointer changes but with a change in > > initialization functions wherein we allocate a connector and encoder > > incase the driver doesn’t have its own this should minimize the effect > > on other drivers already using current drm writeback framework and also > allow i915 to create its own connector. > > I thought that Laurent was quite strict about the connector. So I'd suggest > targeting drm_writeback_connector with an optionally created encoder and > the builtin connector In that case even if we target that i915 will not be able to move forward with its implementation of writeback as builtin connector does not work for us right now as explained earlier, optionally creating encoder and connector will help everyone move forward and once done we can actually sit and work on how to side step this issue using Laurent's suggestion > > > We can work on Laurent's suggestion but that would first require the > > initial RFC patches and then a rework from both of our drivers side to > > see if they gel well with our current code which will take a > > considerable amount of time. We can for now however float the patch > > series up which minimally affects the current drivers and also allows > > i915 and msm to move forward with its writeback implementation off > > course with a proper documentation stating new drivers shouldn't try to > make their own connectors and encoders and that drm_writeback will do > that for them. > > Once that's done and merged we can work with Laurent on his proposal > > to improve the drm writeback framework so that this issue can be side > stepped entirely in future. > > For now I would like to keep the encoder and connector pointer changes > together. Best Regards, Suraj Kandpal
RE: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi Abhinav, > Hi Laurent > > Ok sure, I can take this up but I need to understand the proposal a little bit > more before proceeding on this. So we will discuss this in another email > where we specifically talk about the connector changes. > > Also, I am willing to take this up once the encoder part is done and merged > so that atleast we keep moving on that as MSM WB implementation can > proceed with that first. > > Hi Jani and Suraj > > Any concerns with splitting up the series into encoder and connector OR re- > arranging them? > > Let me know if you can do this OR I can also split this up keeping Suraj's > name in the Co-developed by tag. I was actually thinking of floating a new patch series with both encoder and connector pointer changes but with a change in initialization functions wherein we allocate a connector and encoder incase the driver doesn’t have its own this should minimize the effect on other drivers already using current drm writeback framework and also allow i915 to create its own connector. We can work on Laurent's suggestion but that would first require the initial RFC patches and then a rework from both of our drivers side to see if they gel well with our current code which will take a considerable amount of time. We can for now however float the patch series up which minimally affects the current drivers and also allows i915 and msm to move forward with its writeback implementation off course with a proper documentation stating new drivers shouldn't try to make their own connectors and encoders and that drm_writeback will do that for them. Once that's done and merged we can work with Laurent on his proposal to improve the drm writeback framework so that this issue can be side stepped entirely in future. For now I would like to keep the encoder and connector pointer changes together. Best Regards, Suraj Kandpal
RE: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hi Abhinav, > Based on the discussion in this thread [1] , it seems like having drm_encoder > as a pointer seems to have merits for both of us and also in agreement with > the folks on this thread and has a better chance of an ack. > > However drm_connector is not. > > I had a brief look at your implementation. Any reason you need to go > through intel_connector here and not drm_writeback_connector directly? > > The reason I ask is that I see you are not using prepare_writeback_job hook. > If you use that, you can use the drm_writeback_connector passed on from > there instead of looping through your list like you are doing in > intel_find_writeback_connector. > > Also, none of the other entries of struct intel_connector are being used for > the writeback implementation. So does the drm_writeback_connector in > your implementation need to be an intel_connector when its anyway not > using other fields? Why cant it be just stored as a drm_writeback_connector > itself in your struct intel_wd. The reason we can't do that is i915 driver always assumes that all connectors present in device list is an intel connector and since drm_writeback_connector even though a faux connector in it's initialization calls drm_connector_init() and gets added to the drm device list and hence the i915 driver also expects a corresponding intel connector to go with it. In case I do try to make writeback connector standalone a lot of checks, a lot will have to be added all around the driver as there could be a chance that one of the drm_connector in the drm device list may not be an intel_connector. Yes right now not all fields of intel_connector are being used but we will be working on filling them as we add more functionality to writeback for example introducing content protection. So even if I do float the patch series with just drm_encoder as pointer it won't help us with our implementation if drm_connector isn't a pointer too. Regards, Suraj Kandpal
RE: [PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Hey, > The connector/encoder funcs you do have to pass to > drm_writeback_connector_init() can't use any of the shared driver > infrastructure that assume a certain inheritance. > > See also my reply to Laurent [1]. > > > It well might be that we all misunderstand your design. Do you have a > > complete intel drm_writeback implementation based on this patchset? It > > would be easier to judge if the approach is correct seeing your > > target. > > That would be up to Suraj Kandpal. I have floated the intel drm_writeback implementation. Please refer to [1] to get a better understanding of how we are implementing writeback functionality. [1] https://patchwork.freedesktop.org/series/100617/ Thanks, Suraj Kandpal
RE: [PATCH 1/6] drm: add writeback pointers to drm_connector
Hi Dmitry, Thanks for your comments > Could you please clarify, why do you want to use intel_connector for the > writeback connector? > I can see a usecase for sharing an encoder between the display and writback > pipelines (and if I'm not mistaken, this is the case for Abhinav's case). > However, sharing the hardware-backed connector and writeback connector > sounds like a sign of a loose abstraction for me. > > Please correct me if I'm wrong and Intel driver would really benefit from > reusing intel_connector as a base for drm_writeback_connector. > The thing is drm_writeback_connector contains drm_connector and drm_encoder fields which get initialized when we call drm_writeback_connector_init adding the connector to the list of connectors for the drm_device. Now if I decide not to wrap drm_writeback_connector with intel_connector the issue is I'll have to add a lot of checks all over the driver to see if the drm_connector is actually present inside intel_connector or not. I don’t see the point of not having drm_writeback_ connector as even though it’s a faux connector we still treat is as a connector and it would be better for us to use intel_connector for writeback_connector. Also the current drm_writeback_connector structure kind of restricts drivers consequently dictating how they implement their writeback functionality, as a midlayer I don't feel that would be the right approach as drivers should have the freedom to develop their own flow to implement the functionality and use the midlayer as a helper to get that done. Best Regards, Suraj Kandpal
[PATCH 6/6] drm/arm: changes to malidp driver resulting from drm_writeback_connector structure changes
Changing malidp driver to accomadate the change of
drm_writeback_connector.base and drm_writeback_connector.encoder
to a pointer the reason for which is explained in the
Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_drv.h | 2 ++
drivers/gpu/drm/arm/malidp_mw.c | 12
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index 494075ddbef6..294aacd4beef 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -424,7 +424,7 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
u32 new_mask = crtc_state->connector_mask;
if ((old_mask ^ new_mask) ==
- (1 << drm_connector_index(>mw_connector.base)))
+ (1 << drm_connector_index(malidp->mw_connector.base)))
crtc_state->connectors_changed = false;
}
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index cdfddfabf2d1..971810a685f1 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -31,6 +31,8 @@ struct malidp_error_stats {
struct malidp_drm {
struct malidp_hw_device *dev;
struct drm_crtc crtc;
+ struct drm_connector connector;
+ struct drm_encoder encoder;
struct drm_writeback_connector mw_connector;
wait_queue_head_t wq;
struct drm_pending_vblank_event *event;
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index f5847a79dd7e..9bd2e400cd3d 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -206,21 +206,25 @@ static u32 *get_writeback_formats(struct malidp_drm
*malidp, int *n_formats)
int malidp_mw_connector_init(struct drm_device *drm)
{
struct malidp_drm *malidp = drm->dev_private;
+ struct drm_writeback_connector *wb_conn;
u32 *formats;
int ret, n_formats;
if (!malidp->dev->hw->enable_memwrite)
return 0;
- malidp->mw_connector.encoder.possible_crtcs = 1 <<
drm_crtc_index(>crtc);
- drm_connector_helper_add(>mw_connector.base,
+ wb_conn = >mw_connector;
+ wb_conn->base = >connector;
+ wb_conn->encoder = >encoder;
+ malidp->mw_connector.encoder->possible_crtcs = 1 <<
drm_crtc_index(>crtc);
+ drm_connector_helper_add(wb_conn->base,
_mw_connector_helper_funcs);
formats = get_writeback_formats(malidp, _formats);
if (!formats)
return -ENOMEM;
- ret = drm_writeback_connector_init(drm, >mw_connector,
+ ret = drm_writeback_connector_init(drm, wb_conn,
_mw_connector_funcs,
_mw_encoder_helper_funcs,
formats, n_formats);
@@ -236,7 +240,7 @@ void malidp_mw_atomic_commit(struct drm_device *drm,
{
struct malidp_drm *malidp = drm->dev_private;
struct drm_writeback_connector *mw_conn = >mw_connector;
- struct drm_connector_state *conn_state = mw_conn->base.state;
+ struct drm_connector_state *conn_state = mw_conn->base->state;
struct malidp_hw_device *hwdev = malidp->dev;
struct malidp_mw_connector_state *mw_state;
--
2.17.1
[PATCH 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Changing rcar_du driver to accomadate the change of
drm_writeback_connector.base and drm_writeback_connector.encoder
to a pointer the reason for which is explained in the
Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 ++
drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 8 +---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 66e8839db708..68f387a04502 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -72,6 +72,8 @@ struct rcar_du_crtc {
const char *const *sources;
unsigned int sources_count;
+ struct drm_connector connector;
+ struct drm_encoder encoder;
struct drm_writeback_connector writeback;
};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
index c79d1259e49b..5b1e83380c47 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
@@ -200,8 +200,10 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
{
struct drm_writeback_connector *wb_conn = >writeback;
- wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(>crtc);
- drm_connector_helper_add(_conn->base,
+ wb_conn->base = >connector;
+ wb_conn->encoder = >encoder;
+ wb_conn->encoder->possible_crtcs = 1 << drm_crtc_index(>crtc);
+ drm_connector_helper_add(wb_conn->base,
_du_wb_conn_helper_funcs);
return drm_writeback_connector_init(>ddev, wb_conn,
@@ -220,7 +222,7 @@ void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,
struct drm_framebuffer *fb;
unsigned int i;
- state = rcrtc->writeback.base.state;
+ state = rcrtc->writeback.base->state;
if (!state || !state->writeback_job)
return;
--
2.17.1
[PATCH 4/6] drm/vc4: vc4 driver changes to accommodate changes done in drm_writeback_connector structure
Changing vc4 driver to accomadate the change of
drm_writeback_connector.base and drm_writeback_connector.encoder
to a pointer the reason for which is explained in the
Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/vc4/vc4_txp.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 9809ca3e2945..9882569d147c 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -151,6 +151,10 @@ struct vc4_txp {
struct platform_device *pdev;
+ struct drm_connector drm_conn;
+
+ struct drm_encoder drm_enc;
+
struct drm_writeback_connector connector;
void __iomem *regs;
@@ -159,12 +163,12 @@ struct vc4_txp {
static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder *encoder)
{
- return container_of(encoder, struct vc4_txp, connector.encoder);
+ return container_of(encoder, struct vc4_txp, drm_enc);
}
static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector *conn)
{
- return container_of(conn, struct vc4_txp, connector.base);
+ return container_of(conn, struct vc4_txp, drm_conn);
}
static const struct debugfs_reg32 txp_regs[] = {
@@ -467,6 +471,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
struct vc4_txp *txp;
struct drm_crtc *crtc;
struct drm_encoder *encoder;
+ struct drm_writeback_connector *wb_conn;
int ret, irq;
irq = platform_get_irq(pdev, 0);
@@ -491,10 +496,13 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
txp->regset.base = txp->regs;
txp->regset.regs = txp_regs;
txp->regset.nregs = ARRAY_SIZE(txp_regs);
+ wb_conn = >connector;
+ wb_conn->base = >drm_conn;
+ wb_conn->encoder = >drm_enc;
- drm_connector_helper_add(>connector.base,
+ drm_connector_helper_add(wb_conn->base,
_txp_connector_helper_funcs);
- ret = drm_writeback_connector_init(drm, >connector,
+ ret = drm_writeback_connector_init(drm, wb_conn,
_txp_connector_funcs,
_txp_encoder_helper_funcs,
drm_fmts, ARRAY_SIZE(drm_fmts));
@@ -506,7 +514,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
if (ret)
return ret;
- encoder = >connector.encoder;
+ encoder = txp->connector.encoder;
encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
@@ -529,7 +537,7 @@ static void vc4_txp_unbind(struct device *dev, struct
device *master,
struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_txp *txp = dev_get_drvdata(dev);
- vc4_txp_connector_destroy(>connector.base);
+ vc4_txp_connector_destroy(txp->connector.base);
vc4->txp = NULL;
}
--
2.17.1
[PATCH 3/6] drm/vkms: change vkms driver to use drm_writeback_connector.base pointer
Changing vkms driver to accomadate the change of
drm_writeback_connector.base to a pointer the reason for which is
explained in the Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/vkms/vkms_writeback.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c
b/drivers/gpu/drm/vkms/vkms_writeback.c
index 8694227f555f..374431471f49 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -114,7 +114,7 @@ static void vkms_wb_atomic_commit(struct drm_connector
*conn,
struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
struct vkms_output *output = >output;
struct drm_writeback_connector *wb_conn = >wb_connector;
- struct drm_connector_state *conn_state = wb_conn->base.state;
+ struct drm_connector_state *conn_state = wb_conn->base->state;
struct vkms_crtc_state *crtc_state = output->composer_state;
if (!conn_state)
@@ -139,9 +139,12 @@ static const struct drm_connector_helper_funcs
vkms_wb_conn_helper_funcs = {
int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
{
struct drm_writeback_connector *wb = >output.wb_connector;
+ struct vkms_output *output = >output;
- vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
- drm_connector_helper_add(>base, _wb_conn_helper_funcs);
+ wb->base = >connector;
+ wb->encoder = >encoder;
+ output->wb_connector.encoder->possible_crtcs = 1;
+ drm_connector_helper_add(wb->base, _wb_conn_helper_funcs);
return drm_writeback_connector_init(>drm, wb,
_wb_connector_funcs,
--
2.17.1
[PATCH 2/6] drm/arm/komeda : change driver to use drm_writeback_connector.base pointer
Making changes to komeda driver because we had to change
drm_writeback_connector.base into a pointer the reason for which is
expained in the Patch (drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
Reviewed-by: Carsten Haitzler
---
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +-
drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 ++-
.../gpu/drm/arm/display/komeda/komeda_wb_connector.c | 11 ++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 59172acb9738..eb37f41c1790 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -265,7 +265,7 @@ komeda_crtc_do_flush(struct drm_crtc *crtc,
if (slave && has_bit(slave->id, kcrtc_st->affected_pipes))
komeda_pipeline_update(slave, old->state);
- conn_st = wb_conn ? wb_conn->base.base.state : NULL;
+ conn_st = wb_conn ? wb_conn->base.base->state : NULL;
if (conn_st && conn_st->writeback_job)
drm_writeback_queue_job(_conn->base, conn_st);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 456f3c435719..8d83883a1d99 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -53,6 +53,7 @@ struct komeda_plane_state {
* struct komeda_wb_connector
*/
struct komeda_wb_connector {
+ struct drm_connector conn;
/** @base: _writeback_connector */
struct drm_writeback_connector base;
@@ -136,7 +137,7 @@ struct komeda_kms_dev {
static inline bool is_writeback_only(struct drm_crtc_state *st)
{
struct komeda_wb_connector *wb_conn = to_kcrtc(st->crtc)->wb_conn;
- struct drm_connector *conn = wb_conn ? _conn->base.base : NULL;
+ struct drm_connector *conn = wb_conn ? wb_conn->base.base : NULL;
return conn && (st->connector_mask == BIT(drm_connector_index(conn)));
}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index e465cc4879c9..2c3dec59fd88 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -51,7 +51,7 @@ komeda_wb_encoder_atomic_check(struct drm_encoder *encoder,
return -EINVAL;
}
- wb_layer = to_kconn(to_wb_conn(conn_st->connector))->wb_layer;
+ wb_layer =
to_kconn(drm_connector_to_writeback(conn_st->connector))->wb_layer;
/*
* No need for a full modested when the only connector changed is the
@@ -123,7 +123,7 @@ komeda_wb_connector_fill_modes(struct drm_connector
*connector,
static void komeda_wb_connector_destroy(struct drm_connector *connector)
{
drm_connector_cleanup(connector);
- kfree(to_kconn(to_wb_conn(connector)));
+ kfree(to_kconn(drm_connector_to_writeback(connector)));
}
static const struct drm_connector_funcs komeda_wb_connector_funcs = {
@@ -155,7 +155,8 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
kwb_conn->wb_layer = kcrtc->master->wb_layer;
wb_conn = _conn->base;
- wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(>base));
+ wb_conn->base = _conn->conn;
+ wb_conn->encoder->possible_crtcs = BIT(drm_crtc_index(>base));
formats = komeda_get_layer_fourcc_list(>fmt_tbl,
kwb_conn->wb_layer->layer_type,
@@ -171,9 +172,9 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
return err;
}
- drm_connector_helper_add(_conn->base, _wb_conn_helper_funcs);
+ drm_connector_helper_add(wb_conn->base, _wb_conn_helper_funcs);
- info = _conn->base.base.display_info;
+ info = _conn->base.base->display_info;
info->bpc = __fls(kcrtc->master->improc->supported_color_depths);
info->color_formats = kcrtc->master->improc->supported_color_formats;
--
2.17.1
[PATCH 1/6] drm: add writeback pointers to drm_connector
Changing drm_connector and drm_encoder feilds to pointers in
drm_writeback_connector as the elements of struct
drm_writeback_connector are:
struct drm_writeback_connector {
struct drm_connector base;
struct drm_encoder encoder;
Similarly the elements of intel_encoder and intel_connector
are:
struct intel_encoder {
struct drm_encoder base;
struct intel_connector {
struct drm_connector base;
The function drm_writeback_connector_init() will initialize the
drm_connector and drm_encoder and attach them as well.
Since the drm_connector/encoder are both struct in
drm_writeback_connector and intel_connector/encoder, we need
one of them to be a pointer so we can reference them or else we
will be pointing to 2 seprate instances.
Usually the struct defined in drm framework pointing to any
struct will be pointer and allocating them and initialization
will be done with the users.
Like struct drm_connector and drm_encoder are part of drm
framework and the users of these such as i915 have included them
in their struct intel_connector and intel_encoder. Likewise
struct drm_writeback_connector is a special connector and hence
is not a user of drm_connector and hence this should be pointers.
Adding drm_writeback_connector to drm_connector so that
writeback_connector can be fetched from drm_connector as the previous
container_of method won't work due to change in the feilds of
drm_connector and drm_encoder in drm_writeback_connector.
Note:The corresponding ripple effect due to the above changes namely in
two drivers as I can see it komeda and vkms have been dealt with in the
upcoming patches of this series.
Signed-off-by: Kandpal Suraj
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/drm_writeback.c | 19 ++-
include/drm/drm_connector.h | 3 +++
include/drm/drm_writeback.h | 6 +++---
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504f1bb..47238db42363 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -87,7 +87,7 @@ static const char *drm_writeback_fence_get_driver_name(struct
dma_fence *fence)
struct drm_writeback_connector *wb_connector =
fence_to_wb_connector(fence);
- return wb_connector->base.dev->driver->name;
+ return wb_connector->base->dev->driver->name;
}
static const char *
@@ -177,7 +177,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
const u32 *formats, int n_formats)
{
struct drm_property_blob *blob;
- struct drm_connector *connector = _connector->base;
+ struct drm_connector *connector = wb_connector->base;
struct drm_mode_config *config = >mode_config;
int ret = create_writeback_properties(dev);
@@ -189,14 +189,15 @@ int drm_writeback_connector_init(struct drm_device *dev,
if (IS_ERR(blob))
return PTR_ERR(blob);
- drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
- ret = drm_encoder_init(dev, _connector->encoder,
+ drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
+ ret = drm_encoder_init(dev, wb_connector->encoder,
_writeback_encoder_funcs,
DRM_MODE_ENCODER_VIRTUAL, NULL);
if (ret)
goto fail;
connector->interlace_allowed = 0;
+ connector->wb_connector = wb_connector;
ret = drm_connector_init(dev, connector, con_funcs,
DRM_MODE_CONNECTOR_WRITEBACK);
@@ -204,7 +205,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
goto connector_fail;
ret = drm_connector_attach_encoder(connector,
- _connector->encoder);
+ wb_connector->encoder);
if (ret)
goto attach_fail;
@@ -233,7 +234,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
attach_fail:
drm_connector_cleanup(connector);
connector_fail:
- drm_encoder_cleanup(_connector->encoder);
+ drm_encoder_cleanup(wb_connector->encoder);
fail:
drm_property_blob_put(blob);
return ret;
@@ -263,7 +264,7 @@ int drm_writeback_prepare_job(struct drm_writeback_job *job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
- connector->base.helper_private;
+ connector->base->helper_private;
int ret;
if (funcs->prepare_writeback_job) {
@@ -315,7 +316,7 @@ void drm_writeback_cleanup_job(struct drm_writeback_job
*job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
-
[PATCH 0/6] drm writeback connector changes
This patch series contains drm_writeback_connector structure change i.e change of drm_connector and drm_encoder fields to a pointer the reason for which is explained in patch(1/6) drm: add writeback pointers to drm_connector and the accompanying changes to the different drivers that were effected by it. I had perviously floated a patch series but missed some of the drivers that were effected by the change hence refloating the patch series Kandpal Suraj (6): drm: add writeback pointers to drm_connector drm/arm/komeda : change driver to use drm_writeback_connector.base pointer drm/vkms: change vkms driver to use drm_writeback_connector.base pointer drm/vc4: vc4 driver changes to accommodate changes done in drm_writeback_connector structure drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes drm/arm: changes to malidp driver resulting from drm_writeback_connector structure changes .../gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +- .../gpu/drm/arm/display/komeda/komeda_kms.h | 3 ++- .../arm/display/komeda/komeda_wb_connector.c | 11 +- drivers/gpu/drm/arm/malidp_crtc.c | 2 +- drivers/gpu/drm/arm/malidp_drv.h | 2 ++ drivers/gpu/drm/arm/malidp_mw.c | 12 +++ drivers/gpu/drm/drm_writeback.c | 19 +- drivers/gpu/drm/rcar-du/rcar_du_crtc.h| 2 ++ drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 8 +--- drivers/gpu/drm/vc4/vc4_txp.c | 20 +-- drivers/gpu/drm/vkms/vkms_writeback.c | 9 ++--- include/drm/drm_connector.h | 3 +++ include/drm/drm_writeback.h | 6 +++--- 13 files changed, 63 insertions(+), 36 deletions(-) -- 2.17.1
[PATCH v2 6/6] drm/arm: changes to malidp driver resulting from drm_writeback_connector structure changes
Changing malidp driver to accomadate the change of
drm_writeback_connector.base and drm_writeback_connector.encoder
to a pointer the reason for which is explained in the
Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_drv.h | 2 ++
drivers/gpu/drm/arm/malidp_mw.c | 12
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index 494075ddbef6..294aacd4beef 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -424,7 +424,7 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
u32 new_mask = crtc_state->connector_mask;
if ((old_mask ^ new_mask) ==
- (1 << drm_connector_index(>mw_connector.base)))
+ (1 << drm_connector_index(malidp->mw_connector.base)))
crtc_state->connectors_changed = false;
}
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index cdfddfabf2d1..971810a685f1 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -31,6 +31,8 @@ struct malidp_error_stats {
struct malidp_drm {
struct malidp_hw_device *dev;
struct drm_crtc crtc;
+ struct drm_connector connector;
+ struct drm_encoder encoder;
struct drm_writeback_connector mw_connector;
wait_queue_head_t wq;
struct drm_pending_vblank_event *event;
diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
index f5847a79dd7e..9bd2e400cd3d 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -206,21 +206,25 @@ static u32 *get_writeback_formats(struct malidp_drm
*malidp, int *n_formats)
int malidp_mw_connector_init(struct drm_device *drm)
{
struct malidp_drm *malidp = drm->dev_private;
+ struct drm_writeback_connector *wb_conn;
u32 *formats;
int ret, n_formats;
if (!malidp->dev->hw->enable_memwrite)
return 0;
- malidp->mw_connector.encoder.possible_crtcs = 1 <<
drm_crtc_index(>crtc);
- drm_connector_helper_add(>mw_connector.base,
+ wb_conn = >mw_connector;
+ wb_conn->base = >connector;
+ wb_conn->encoder = >encoder;
+ malidp->mw_connector.encoder->possible_crtcs = 1 <<
drm_crtc_index(>crtc);
+ drm_connector_helper_add(wb_conn->base,
_mw_connector_helper_funcs);
formats = get_writeback_formats(malidp, _formats);
if (!formats)
return -ENOMEM;
- ret = drm_writeback_connector_init(drm, >mw_connector,
+ ret = drm_writeback_connector_init(drm, wb_conn,
_mw_connector_funcs,
_mw_encoder_helper_funcs,
formats, n_formats);
@@ -236,7 +240,7 @@ void malidp_mw_atomic_commit(struct drm_device *drm,
{
struct malidp_drm *malidp = drm->dev_private;
struct drm_writeback_connector *mw_conn = >mw_connector;
- struct drm_connector_state *conn_state = mw_conn->base.state;
+ struct drm_connector_state *conn_state = mw_conn->base->state;
struct malidp_hw_device *hwdev = malidp->dev;
struct malidp_mw_connector_state *mw_state;
--
2.17.1
[PATCH v2 5/6] drm/rcar_du: changes to rcar-du driver resulting from drm_writeback_connector structure changes
Changing rcar_du driver to accomadate the change of
drm_writeback_connector.base and drm_writeback_connector.encoder
to a pointer the reason for which is explained in the
Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 ++
drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 8 +---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 66e8839db708..68f387a04502 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -72,6 +72,8 @@ struct rcar_du_crtc {
const char *const *sources;
unsigned int sources_count;
+ struct drm_connector connector;
+ struct drm_encoder encoder;
struct drm_writeback_connector writeback;
};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
index c79d1259e49b..5b1e83380c47 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
@@ -200,8 +200,10 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu,
{
struct drm_writeback_connector *wb_conn = >writeback;
- wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(>crtc);
- drm_connector_helper_add(_conn->base,
+ wb_conn->base = >connector;
+ wb_conn->encoder = >encoder;
+ wb_conn->encoder->possible_crtcs = 1 << drm_crtc_index(>crtc);
+ drm_connector_helper_add(wb_conn->base,
_du_wb_conn_helper_funcs);
return drm_writeback_connector_init(>ddev, wb_conn,
@@ -220,7 +222,7 @@ void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc,
struct drm_framebuffer *fb;
unsigned int i;
- state = rcrtc->writeback.base.state;
+ state = rcrtc->writeback.base->state;
if (!state || !state->writeback_job)
return;
--
2.17.1
[PATCH v2 4/6] drm/vc4: vc4 driver changes to accommodate changes done in drm_writeback_connector structure
Changing vc4 driver to accomadate the change of
drm_writeback_connector.base and drm_writeback_connector.encoder
to a pointer the reason for which is explained in the
Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/vc4/vc4_txp.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 9809ca3e2945..9882569d147c 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -151,6 +151,10 @@ struct vc4_txp {
struct platform_device *pdev;
+ struct drm_connector drm_conn;
+
+ struct drm_encoder drm_enc;
+
struct drm_writeback_connector connector;
void __iomem *regs;
@@ -159,12 +163,12 @@ struct vc4_txp {
static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder *encoder)
{
- return container_of(encoder, struct vc4_txp, connector.encoder);
+ return container_of(encoder, struct vc4_txp, drm_enc);
}
static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector *conn)
{
- return container_of(conn, struct vc4_txp, connector.base);
+ return container_of(conn, struct vc4_txp, drm_conn);
}
static const struct debugfs_reg32 txp_regs[] = {
@@ -467,6 +471,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
struct vc4_txp *txp;
struct drm_crtc *crtc;
struct drm_encoder *encoder;
+ struct drm_writeback_connector *wb_conn;
int ret, irq;
irq = platform_get_irq(pdev, 0);
@@ -491,10 +496,13 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
txp->regset.base = txp->regs;
txp->regset.regs = txp_regs;
txp->regset.nregs = ARRAY_SIZE(txp_regs);
+ wb_conn = >connector;
+ wb_conn->base = >drm_conn;
+ wb_conn->encoder = >drm_enc;
- drm_connector_helper_add(>connector.base,
+ drm_connector_helper_add(wb_conn->base,
_txp_connector_helper_funcs);
- ret = drm_writeback_connector_init(drm, >connector,
+ ret = drm_writeback_connector_init(drm, wb_conn,
_txp_connector_funcs,
_txp_encoder_helper_funcs,
drm_fmts, ARRAY_SIZE(drm_fmts));
@@ -506,7 +514,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
if (ret)
return ret;
- encoder = >connector.encoder;
+ encoder = txp->connector.encoder;
encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
@@ -529,7 +537,7 @@ static void vc4_txp_unbind(struct device *dev, struct
device *master,
struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_txp *txp = dev_get_drvdata(dev);
- vc4_txp_connector_destroy(>connector.base);
+ vc4_txp_connector_destroy(txp->connector.base);
vc4->txp = NULL;
}
--
2.17.1
[PATCH v2 3/6] drm/vkms: change vkms driver to use drm_writeback_connector.base pointer
Changing vkms driver to accomadate the change of
drm_writeback_connector.base to a pointer the reason for which is
explained in the Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/vkms/vkms_writeback.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c
b/drivers/gpu/drm/vkms/vkms_writeback.c
index 8694227f555f..374431471f49 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -114,7 +114,7 @@ static void vkms_wb_atomic_commit(struct drm_connector
*conn,
struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
struct vkms_output *output = >output;
struct drm_writeback_connector *wb_conn = >wb_connector;
- struct drm_connector_state *conn_state = wb_conn->base.state;
+ struct drm_connector_state *conn_state = wb_conn->base->state;
struct vkms_crtc_state *crtc_state = output->composer_state;
if (!conn_state)
@@ -139,9 +139,12 @@ static const struct drm_connector_helper_funcs
vkms_wb_conn_helper_funcs = {
int vkms_enable_writeback_connector(struct vkms_device *vkmsdev)
{
struct drm_writeback_connector *wb = >output.wb_connector;
+ struct vkms_output *output = >output;
- vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
- drm_connector_helper_add(>base, _wb_conn_helper_funcs);
+ wb->base = >connector;
+ wb->encoder = >encoder;
+ output->wb_connector.encoder->possible_crtcs = 1;
+ drm_connector_helper_add(wb->base, _wb_conn_helper_funcs);
return drm_writeback_connector_init(>drm, wb,
_wb_connector_funcs,
--
2.17.1
[PATCH v2 2/6] drm/arm/komeda : change driver to use drm_writeback_connector.base pointer
Making changes to komeda driver because we had to change
drm_writeback_connector.base into a pointer the reason for which is
expained in the Patch (drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +-
drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 ++-
.../gpu/drm/arm/display/komeda/komeda_wb_connector.c | 11 ++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 59172acb9738..eb37f41c1790 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -265,7 +265,7 @@ komeda_crtc_do_flush(struct drm_crtc *crtc,
if (slave && has_bit(slave->id, kcrtc_st->affected_pipes))
komeda_pipeline_update(slave, old->state);
- conn_st = wb_conn ? wb_conn->base.base.state : NULL;
+ conn_st = wb_conn ? wb_conn->base.base->state : NULL;
if (conn_st && conn_st->writeback_job)
drm_writeback_queue_job(_conn->base, conn_st);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 456f3c435719..8d83883a1d99 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -53,6 +53,7 @@ struct komeda_plane_state {
* struct komeda_wb_connector
*/
struct komeda_wb_connector {
+ struct drm_connector conn;
/** @base: _writeback_connector */
struct drm_writeback_connector base;
@@ -136,7 +137,7 @@ struct komeda_kms_dev {
static inline bool is_writeback_only(struct drm_crtc_state *st)
{
struct komeda_wb_connector *wb_conn = to_kcrtc(st->crtc)->wb_conn;
- struct drm_connector *conn = wb_conn ? _conn->base.base : NULL;
+ struct drm_connector *conn = wb_conn ? wb_conn->base.base : NULL;
return conn && (st->connector_mask == BIT(drm_connector_index(conn)));
}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index e465cc4879c9..2c3dec59fd88 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -51,7 +51,7 @@ komeda_wb_encoder_atomic_check(struct drm_encoder *encoder,
return -EINVAL;
}
- wb_layer = to_kconn(to_wb_conn(conn_st->connector))->wb_layer;
+ wb_layer =
to_kconn(drm_connector_to_writeback(conn_st->connector))->wb_layer;
/*
* No need for a full modested when the only connector changed is the
@@ -123,7 +123,7 @@ komeda_wb_connector_fill_modes(struct drm_connector
*connector,
static void komeda_wb_connector_destroy(struct drm_connector *connector)
{
drm_connector_cleanup(connector);
- kfree(to_kconn(to_wb_conn(connector)));
+ kfree(to_kconn(drm_connector_to_writeback(connector)));
}
static const struct drm_connector_funcs komeda_wb_connector_funcs = {
@@ -155,7 +155,8 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
kwb_conn->wb_layer = kcrtc->master->wb_layer;
wb_conn = _conn->base;
- wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(>base));
+ wb_conn->base = _conn->conn;
+ wb_conn->encoder->possible_crtcs = BIT(drm_crtc_index(>base));
formats = komeda_get_layer_fourcc_list(>fmt_tbl,
kwb_conn->wb_layer->layer_type,
@@ -171,9 +172,9 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
return err;
}
- drm_connector_helper_add(_conn->base, _wb_conn_helper_funcs);
+ drm_connector_helper_add(wb_conn->base, _wb_conn_helper_funcs);
- info = _conn->base.base.display_info;
+ info = _conn->base.base->display_info;
info->bpc = __fls(kcrtc->master->improc->supported_color_depths);
info->color_formats = kcrtc->master->improc->supported_color_formats;
--
2.17.1
[PATCH v2 1/6] drm: add writeback pointers to drm_connector
Changing drm_connector and drm_encoder feilds to pointers in
drm_writeback_connector as the elements of struct
drm_writeback_connector are:
struct drm_writeback_connector {
struct drm_connector base;
struct drm_encoder encoder;
Similarly the elements of intel_encoder and intel_connector
are:
struct intel_encoder {
struct drm_encoder base;
struct intel_connector {
struct drm_connector base;
The function drm_writeback_connector_init() will initialize the
drm_connector and drm_encoder and attach them as well.
Since the drm_connector/encoder are both struct in
drm_writeback_connector and intel_connector/encoder, we need
one of them to be a pointer so we can reference them or else we
will be pointing to 2 seprate instances.
Usually the struct defined in drm framework pointing to any
struct will be pointer and allocating them and initialization
will be done with the users.
Like struct drm_connector and drm_encoder are part of drm
framework and the users of these such as i915 have included them
in their struct intel_connector and intel_encoder. Likewise
struct drm_writeback_connector is a special connector and hence
is not a user of drm_connector and hence this should be pointers.
Adding drm_writeback_connector to drm_connector so that
writeback_connector can be fetched from drm_connector as the previous
container_of method won't work due to change in the feilds of
drm_connector and drm_encoder in drm_writeback_connector.
Note:The corresponding ripple effect due to the above changes namely in
two drivers as I can see it komeda and vkms have been dealt with in the
upcoming patches of this series.
Signed-off-by: Kandpal Suraj
---
drivers/gpu/drm/drm_writeback.c | 19 ++-
include/drm/drm_connector.h | 3 +++
include/drm/drm_writeback.h | 6 +++---
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504f1bb..47238db42363 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -87,7 +87,7 @@ static const char *drm_writeback_fence_get_driver_name(struct
dma_fence *fence)
struct drm_writeback_connector *wb_connector =
fence_to_wb_connector(fence);
- return wb_connector->base.dev->driver->name;
+ return wb_connector->base->dev->driver->name;
}
static const char *
@@ -177,7 +177,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
const u32 *formats, int n_formats)
{
struct drm_property_blob *blob;
- struct drm_connector *connector = _connector->base;
+ struct drm_connector *connector = wb_connector->base;
struct drm_mode_config *config = >mode_config;
int ret = create_writeback_properties(dev);
@@ -189,14 +189,15 @@ int drm_writeback_connector_init(struct drm_device *dev,
if (IS_ERR(blob))
return PTR_ERR(blob);
- drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
- ret = drm_encoder_init(dev, _connector->encoder,
+ drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
+ ret = drm_encoder_init(dev, wb_connector->encoder,
_writeback_encoder_funcs,
DRM_MODE_ENCODER_VIRTUAL, NULL);
if (ret)
goto fail;
connector->interlace_allowed = 0;
+ connector->wb_connector = wb_connector;
ret = drm_connector_init(dev, connector, con_funcs,
DRM_MODE_CONNECTOR_WRITEBACK);
@@ -204,7 +205,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
goto connector_fail;
ret = drm_connector_attach_encoder(connector,
- _connector->encoder);
+ wb_connector->encoder);
if (ret)
goto attach_fail;
@@ -233,7 +234,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
attach_fail:
drm_connector_cleanup(connector);
connector_fail:
- drm_encoder_cleanup(_connector->encoder);
+ drm_encoder_cleanup(wb_connector->encoder);
fail:
drm_property_blob_put(blob);
return ret;
@@ -263,7 +264,7 @@ int drm_writeback_prepare_job(struct drm_writeback_job *job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
- connector->base.helper_private;
+ connector->base->helper_private;
int ret;
if (funcs->prepare_writeback_job) {
@@ -315,7 +316,7 @@ void drm_writeback_cleanup_job(struct drm_writeback_job
*job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
- connector->base.h
RE: [PATCH 1/3] drm: add writeback pointers to drm_connector
Hey,
> I think there are more places affected with this change. I can get below
> compilation issues while trying to compile my branch:
>
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘encoder_to_vc4_txp’:
> ./include/linux/build_bug.h:78:41: error: static assertion failed:
> "pointer type mismatch in container_of()"
> #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
> ^
> ./include/linux/build_bug.h:77:34: note: in expansion of macro
> ‘__static_assert’
> #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__,
> #expr)
>^
> ./include/linux/container_of.h:19:2: note: in expansion of macro
> ‘static_assert’
>static_assert(__same_type(*(ptr), ((type *)0)->member) || \
>^
> drivers/gpu/drm/vc4/vc4_txp.c:162:9: note: in expansion of macro
> ‘container_of’
>return container_of(encoder, struct vc4_txp, connector.encoder);
> ^
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘connector_to_vc4_txp’:
> ./include/linux/build_bug.h:78:41: error: static assertion failed:
> "pointer type mismatch in container_of()"
> #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
> ^
> ./include/linux/build_bug.h:77:34: note: in expansion of macro
> ‘__static_assert’
> #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__,
> #expr)
>^
> ./include/linux/container_of.h:19:2: note: in expansion of macro
> ‘static_assert’
>static_assert(__same_type(*(ptr), ((type *)0)->member) || \
>^
> drivers/gpu/drm/vc4/vc4_txp.c:167:9: note: in expansion of macro
> ‘container_of’
>return container_of(conn, struct vc4_txp, connector.base);
> ^
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_bind’:
> drivers/gpu/drm/vc4/vc4_txp.c:495:27: error: passing argument 1 of
> ‘drm_connector_helper_add’ from incompatible pointer type [-
> Werror=incompatible-pointer-types]
>drm_connector_helper_add(>connector.base,
> ^
> In file included from ./include/drm/drm_atomic_helper.h:32:0,
> from drivers/gpu/drm/vc4/vc4_txp.c:17:
> ./include/drm/drm_modeset_helper_vtables.h:1153:20: note: expected
> ‘struct drm_connector *’ but argument is of type ‘struct drm_connector **’
> static inline void drm_connector_helper_add(struct drm_connector
> *connector,
> ^
> drivers/gpu/drm/vc4/vc4_txp.c:509:10: error: assignment from incompatible
> pointer type [-Werror=incompatible-pointer-types]
>encoder = >connector.encoder;
>^
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_unbind’:
> drivers/gpu/drm/vc4/vc4_txp.c:532:28: error: passing argument 1 of
> ‘vc4_txp_connector_destroy’ from incompatible pointer type [-
> Werror=incompatible-pointer-types]
>vc4_txp_connector_destroy(>connector.base);
> ^
> drivers/gpu/drm/vc4/vc4_txp.c:333:13: note: expected ‘struct
> drm_connector *’ but argument is of type ‘struct drm_connector **’
> static void vc4_txp_connector_destroy(struct drm_connector *connector)
>
I will have a look at this and try to resolve it
>
> Looks like vc4 doesnt have its own encoder so we might have to move it to
> have its own?
Right seems like we will have to add a drm_connector and drm_encoder in the
below structure
>
> struct vc4_txp {
> struct vc4_crtc base;
>
> struct platform_device *pdev;
>
> struct drm_writeback_connector connector;
>
> void __iomem *regs;
> struct debugfs_regset32 regset;
> };
>
> static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder
> *encoder)
> {
> return container_of(encoder, struct vc4_txp, connector.encoder); }
>
> static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector
> *conn)
> {
> return container_of(conn, struct vc4_txp, connector.base); }
>
Changes will be required here in both connector_of functions to point to
The new drm_connector and drm_encoder we add in vc4_txp struct.
>
> One more thing, it looks like to me, we need to do one more thing.
>
> Like intel does and what MSM will do, the encoder pointer of the wb
> connector has to point to the encoder struct .
>
> >> wb_conn = _connector->wb_conn;
> >> wb_conn->base = _connector->base;
> >> wb_conn->encoder = _wd->base.base;
>
Yes this will need to be done
Thanks Abhinav for pointing Ill implement this and send it in the next version
of patches
Regards,
Suraj Kandpal
RE: [PATCH 3/3] drm/vkms: change vkms driver to use drm_writeback_connector.base pointer
Hi All,
Gentle Reminder! Any Review comments?
> Changing vkms driver to accomadate the change of
> drm_writeback_connector.base to a pointer the reason for which is
> explained in the Patch(drm: add writeback pointers to drm_connector).
>
> Signed-off-by: Kandpal, Suraj
> ---
> drivers/gpu/drm/vkms/vkms_writeback.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c
> b/drivers/gpu/drm/vkms/vkms_writeback.c
> index 8694227f555f..6de0c4213425 100644
> --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> @@ -114,7 +114,7 @@ static void vkms_wb_atomic_commit(struct
> drm_connector *conn,
> struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn-
> >dev);
> struct vkms_output *output = >output;
> struct drm_writeback_connector *wb_conn =
> >wb_connector;
> - struct drm_connector_state *conn_state = wb_conn->base.state;
> + struct drm_connector_state *conn_state = wb_conn->base->state;
> struct vkms_crtc_state *crtc_state = output->composer_state;
>
> if (!conn_state)
> @@ -140,8 +140,8 @@ int vkms_enable_writeback_connector(struct
> vkms_device *vkmsdev) {
> struct drm_writeback_connector *wb =
> >output.wb_connector;
>
> - vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
> - drm_connector_helper_add(>base,
> _wb_conn_helper_funcs);
> + vkmsdev->output.wb_connector.encoder->possible_crtcs = 1;
> + drm_connector_helper_add(wb->base,
> _wb_conn_helper_funcs);
>
> return drm_writeback_connector_init(>drm, wb,
> _wb_connector_funcs,
Regards,
Suraj Kandpal
RE: [PATCH 1/3] drm: add writeback pointers to drm_connector
>
> + laurent on this
>
> Hi Suraj
> Jani pointed me to this thread as i had posted something similar here :
> https://patchwork.freedesktop.org/patch/470296/ but since this thread was
> posted earlier, we can discuss further here.
>
> Overall, its similar to what I had posted in the RFC and your commit text also
> covers my concerns too.
>
> One question I have about your change is since you have changed
> wb_connector::encoder to be a pointer, i saw the other changes in the series
> but they do not allocate an encoder. Would this not affect the other drivers
> which are assuming that the encoder in wb_connector is struct drm_encoder
> encoder and not struct drm_encoder* encoder.
>
> Your changes fix the compilation issue but wouldnt this crash as encoder
> wasnt allocated for other drivers.
>
Hi Abhinav,
That shouldn't be an issue as normally drivers tend to have their own output
structure which has drm_connector and drm_encoder embedded in it depending
on the level of binding they have decided to give the connector and encoder in
their respective output and the addresses of these are passed to the
drm_connector* and drm_encoder* in drm_writeback_connector structure
which then gets initialized in drm_writeback_connector_init().
In our i915 code we have intel_connector structure with drm_connector base
field and intel_wd with a intel_encoder base which in turn has drm_encoder
field and both intel_connector and intel_wd are initialized not requiring
explicit
alloc and dealloc for drm_encoder
intel_wd = kzalloc(sizeof(*intel_wd), GFP_KERNEL);
intel_connector = intel_connector_alloc();
wb_conn = _connector->wb_conn;
wb_conn->base = _connector->base;
wb_conn->encoder = _wd->base.base;
Similary for komeda driver has
struct komeda_wb_connector {
struct drm_connector conn;
/** @base: _writeback_connector */
struct drm_writeback_connector base;
/** @wb_layer: represents associated writeback pipeline of komeda */
struct komeda_layer *wb_layer;
};
static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
struct komeda_crtc *kcrtc)
{
struct komeda_wb_connector *kwb_conn;
struct drm_writeback_connector *wb_conn;
kwb_conn = kzalloc(sizeof(*kwb_conn), GFP_KERNEL);
wb_conn = _conn->base;
wb_conn->base = _conn->conn;
and they do not depend on the encoder binding so changes will work for them
Also in vkms driver we have the
struct vkms_output {
struct drm_crtc crtc;
struct drm_encoder encoder;
struct drm_connector connector;
struct drm_writeback_connector wb_connector;
struct hrtimer vblank_hrtimer;
ktime_t period_ns;
struct drm_pending_vblank_event *event;
/* ordered wq for composer_work */
struct workqueue_struct *composer_workq;
/* protects concurrent access to composer */
spinlock_t lock;
/* protected by @lock */
bool composer_enabled;
struct vkms_crtc_state *composer_state;
spinlock_t composer_lock;
};
Which gets allocated covering for the drm_encoder alloc and dealloc
Regards,
Suraj Kandpal
[PATCH 2/3] drm/arm/komeda : change driver to use drm_writeback_connector.base pointer
Making changes to komeda driver because we had to change
drm_writeback_connector.base into a pointer the reason for which is
expained in the Patch (drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal, Suraj
---
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 2 +-
drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 3 ++-
drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 9 +
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 59172acb9738..eb37f41c1790 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -265,7 +265,7 @@ komeda_crtc_do_flush(struct drm_crtc *crtc,
if (slave && has_bit(slave->id, kcrtc_st->affected_pipes))
komeda_pipeline_update(slave, old->state);
- conn_st = wb_conn ? wb_conn->base.base.state : NULL;
+ conn_st = wb_conn ? wb_conn->base.base->state : NULL;
if (conn_st && conn_st->writeback_job)
drm_writeback_queue_job(_conn->base, conn_st);
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 456f3c435719..8d83883a1d99 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -53,6 +53,7 @@ struct komeda_plane_state {
* struct komeda_wb_connector
*/
struct komeda_wb_connector {
+ struct drm_connector conn;
/** @base: _writeback_connector */
struct drm_writeback_connector base;
@@ -136,7 +137,7 @@ struct komeda_kms_dev {
static inline bool is_writeback_only(struct drm_crtc_state *st)
{
struct komeda_wb_connector *wb_conn = to_kcrtc(st->crtc)->wb_conn;
- struct drm_connector *conn = wb_conn ? _conn->base.base : NULL;
+ struct drm_connector *conn = wb_conn ? wb_conn->base.base : NULL;
return conn && (st->connector_mask == BIT(drm_connector_index(conn)));
}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index e465cc4879c9..0caaf483276d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -51,7 +51,7 @@ komeda_wb_encoder_atomic_check(struct drm_encoder *encoder,
return -EINVAL;
}
- wb_layer = to_kconn(to_wb_conn(conn_st->connector))->wb_layer;
+ wb_layer =
to_kconn(drm_connector_to_writeback(conn_st->connector))->wb_layer;
/*
* No need for a full modested when the only connector changed is the
@@ -123,7 +123,7 @@ komeda_wb_connector_fill_modes(struct drm_connector
*connector,
static void komeda_wb_connector_destroy(struct drm_connector *connector)
{
drm_connector_cleanup(connector);
- kfree(to_kconn(to_wb_conn(connector)));
+ kfree(to_kconn(drm_connector_to_writeback(connector)));
}
static const struct drm_connector_funcs komeda_wb_connector_funcs = {
@@ -155,6 +155,7 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
kwb_conn->wb_layer = kcrtc->master->wb_layer;
wb_conn = _conn->base;
+ wb_conn->base = _conn->conn;
wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(>base));
formats = komeda_get_layer_fourcc_list(>fmt_tbl,
@@ -171,9 +172,9 @@ static int komeda_wb_connector_add(struct komeda_kms_dev
*kms,
return err;
}
- drm_connector_helper_add(_conn->base, _wb_conn_helper_funcs);
+ drm_connector_helper_add(wb_conn->base, _wb_conn_helper_funcs);
- info = _conn->base.base.display_info;
+ info = _conn->base.base->display_info;
info->bpc = __fls(kcrtc->master->improc->supported_color_depths);
info->color_formats = kcrtc->master->improc->supported_color_formats;
--
2.17.1
[PATCH 3/3] drm/vkms: change vkms driver to use drm_writeback_connector.base pointer
Changing vkms driver to accomadate the change of
drm_writeback_connector.base to a pointer the reason for which is
explained in the Patch(drm: add writeback pointers to drm_connector).
Signed-off-by: Kandpal, Suraj
---
drivers/gpu/drm/vkms/vkms_writeback.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c
b/drivers/gpu/drm/vkms/vkms_writeback.c
index 8694227f555f..6de0c4213425 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -114,7 +114,7 @@ static void vkms_wb_atomic_commit(struct drm_connector
*conn,
struct vkms_device *vkmsdev = drm_device_to_vkms_device(conn->dev);
struct vkms_output *output = >output;
struct drm_writeback_connector *wb_conn = >wb_connector;
- struct drm_connector_state *conn_state = wb_conn->base.state;
+ struct drm_connector_state *conn_state = wb_conn->base->state;
struct vkms_crtc_state *crtc_state = output->composer_state;
if (!conn_state)
@@ -140,8 +140,8 @@ int vkms_enable_writeback_connector(struct vkms_device
*vkmsdev)
{
struct drm_writeback_connector *wb = >output.wb_connector;
- vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
- drm_connector_helper_add(>base, _wb_conn_helper_funcs);
+ vkmsdev->output.wb_connector.encoder->possible_crtcs = 1;
+ drm_connector_helper_add(wb->base, _wb_conn_helper_funcs);
return drm_writeback_connector_init(>drm, wb,
_wb_connector_funcs,
--
2.17.1
[PATCH 1/3] drm: add writeback pointers to drm_connector
Changing drm_connector and drm_encoder feilds to pointers in
drm_writeback_connector as the elements of struct
drm_writeback_connector are:
struct drm_writeback_connector {
struct drm_connector base;
struct drm_encoder encoder;
Similarly the elements of intel_encoder and intel_connector
are:
struct intel_encoder {
struct drm_encoder base;
struct intel_connector {
struct drm_connector base;
The function drm_writeback_connector_init() will initialize the
drm_connector and drm_encoder and attach them as well.
Since the drm_connector/encoder are both struct in
drm_writeback_connector and intel_connector/encoder, we need
one of them to be a pointer so we can reference them or else we
will be pointing to 2 seprate instances.
Usually the struct defined in drm framework pointing to any
struct will be pointer and allocating them and initialization
will be done with the users.
Like struct drm_connector and drm_encoder are part of drm
framework and the users of these such as i915 have included them
in their struct intel_connector and intel_encoder. Likewise
struct drm_writeback_connector is a special connector and hence
is not a user of drm_connector and hence this should be pointers.
Adding drm_writeback_connector to drm_connector so that
writeback_connector can be fetched from drm_connector as the previous
container_of method won't work due to change in the feilds of
drm_connector and drm_encoder in drm_writeback_connector.
Note:The corresponding ripple effect due to the above changes namely in
two drivers as I can see it komeda and vkms have been dealt with in the
upcoming patches of this series.
Signed-off-by: Kandpal, Suraj
---
drivers/gpu/drm/drm_writeback.c | 19 ++-
include/drm/drm_connector.h | 3 +++
include/drm/drm_writeback.h | 6 +++---
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504f1bb..47238db42363 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -87,7 +87,7 @@ static const char *drm_writeback_fence_get_driver_name(struct
dma_fence *fence)
struct drm_writeback_connector *wb_connector =
fence_to_wb_connector(fence);
- return wb_connector->base.dev->driver->name;
+ return wb_connector->base->dev->driver->name;
}
static const char *
@@ -177,7 +177,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
const u32 *formats, int n_formats)
{
struct drm_property_blob *blob;
- struct drm_connector *connector = _connector->base;
+ struct drm_connector *connector = wb_connector->base;
struct drm_mode_config *config = >mode_config;
int ret = create_writeback_properties(dev);
@@ -189,14 +189,15 @@ int drm_writeback_connector_init(struct drm_device *dev,
if (IS_ERR(blob))
return PTR_ERR(blob);
- drm_encoder_helper_add(_connector->encoder, enc_helper_funcs);
- ret = drm_encoder_init(dev, _connector->encoder,
+ drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
+ ret = drm_encoder_init(dev, wb_connector->encoder,
_writeback_encoder_funcs,
DRM_MODE_ENCODER_VIRTUAL, NULL);
if (ret)
goto fail;
connector->interlace_allowed = 0;
+ connector->wb_connector = wb_connector;
ret = drm_connector_init(dev, connector, con_funcs,
DRM_MODE_CONNECTOR_WRITEBACK);
@@ -204,7 +205,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
goto connector_fail;
ret = drm_connector_attach_encoder(connector,
- _connector->encoder);
+ wb_connector->encoder);
if (ret)
goto attach_fail;
@@ -233,7 +234,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
attach_fail:
drm_connector_cleanup(connector);
connector_fail:
- drm_encoder_cleanup(_connector->encoder);
+ drm_encoder_cleanup(wb_connector->encoder);
fail:
drm_property_blob_put(blob);
return ret;
@@ -263,7 +264,7 @@ int drm_writeback_prepare_job(struct drm_writeback_job *job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
- connector->base.helper_private;
+ connector->base->helper_private;
int ret;
if (funcs->prepare_writeback_job) {
@@ -315,7 +316,7 @@ void drm_writeback_cleanup_job(struct drm_writeback_job
*job)
{
struct drm_writeback_connector *connector = job->connector;
const struct drm_connector_helper_funcs *funcs =
- c
