Re: [Freedreno] [PATCH] arm64: dts: qcom: enable dual ("bonded") DSI mode for DB845c
On Thu, 4 May 2023 19:04:30 +0300, Dmitry Baryshkov wrote: > Now as both lt9611 and drm/msm drivers were updated to handle the 4k > modes over DSI, enable "bonded" DSI mode on DB845c. This way the board > utilizes both DSI links and thus can support 4k on the HDMI output. > > Applied, thanks! [1/1] arm64: dts: qcom: enable dual ("bonded") DSI mode for DB845c commit: 8721e18ca6960f3c5a6a7f58245d9ab084ad09dd Best regards, -- Bjorn Andersson
Re: [Freedreno] [RFC PATCH v2 11/13] drm/msm/dpu: add a field describing inline rotation to dpu_caps
On Thu, 25 May 2023 at 02:20, Abhinav Kumar wrote: > > > > On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: > > We need to know if the platform supports inline rotation on any of the > > SSPP blocks or not. Add this information to struct dpu_caps in a form of > > the boolean field has_inline_rot. > > > > So even for this one, will a helper to detect it from the list of sspps > be better? > > We are now duplicating information from sspp to dpu caps for convenience > and nothing wrong with it if the helper will get called like every > atomic check . > > But looking at the next patch, it seems we use this information only > once during dpu_plane_init(), so why not add a helper to find this out? Sure, why not. > > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > index 2d6944a9679a..33527ec7c938 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > @@ -489,6 +489,7 @@ static const struct dpu_caps sc7280_dpu_caps = { > > .ubwc_version = DPU_HW_UBWC_VER_30, > > .has_dim_layer = true, > > .has_idle_pc = true, > > + .has_inline_rot = true, > > .max_linewidth = 2400, > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > .format_list = plane_formats_yuv, > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > > index 4847aae78db2..cc64fb2e815f 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > > @@ -400,6 +400,7 @@ struct dpu_rotation_cfg { > >* @has_dim_layer dim layer feature status > >* @has_idle_pcindicate if idle power collapse feature is > > supported > >* @has_3d_merge indicate if 3D merge is supported > > + * @has_inline_rot indicate if inline rotation is supported > >* @max_linewidth max linewidth for sspp > >* @pixel_ram_size size of latency hiding and de-tiling buffer in > > bytes > >* @max_hdeci_exp max horizontal decimation supported (max is > > 2^value) > > @@ -416,6 +417,7 @@ struct dpu_caps { > > bool has_dim_layer; > > bool has_idle_pc; > > bool has_3d_merge; > > + bool has_inline_rot; > > /* SSPP limits */ > > u32 max_linewidth; > > u32 pixel_ram_size; -- With best wishes Dmitry
Re: [Freedreno] [RFC PATCH v2 10/13] drm/msm/dpu: add list of supported formats to the DPU caps
On Thu, 25 May 2023 at 02:16, Abhinav Kumar wrote: > > > > On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: > > As we are going to add virtual planes, add the list of supported formats > > to the hw catalog entry. It will be used to setup universal planes, with > > later selecting a pipe depending on whether the YUV format is used for > > the framebuffer. > > > > If your usage of format_list is going to be internal to dpu_plane.c, I > can think of another idea for this change. > > This essentially translates to if (num_vig >= 1) > > If we can just have a small helper to detect that from the catalog can > we use that instead of adding formats to the dpu caps? I'd prefer to be explicit here. The list of supported formats might vary between generations, might it not? Also we don't have a case of the devices not having VIG planes. Even the qcm2290 (which doesn't have CSC) lists YUV as supported. Note: I think at some point we should have a closer look at the list of supported formats and crosscheck that we do not have either unsupported formats being listed, or missing formats which are not listed as supported. > > > Signed-off-by: Dmitry Baryshkov > > --- > > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 26 +++ > > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 4 +++ > > 2 files changed, 30 insertions(+) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > index 212d546b6c5d..2d6944a9679a 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > > @@ -315,6 +315,8 @@ static const struct dpu_caps msm8998_dpu_caps = { > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > .max_hdeci_exp = MAX_HORZ_DECIMATION, > > .max_vdeci_exp = MAX_VERT_DECIMATION, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps qcm2290_dpu_caps = { > > @@ -324,6 +326,8 @@ static const struct dpu_caps qcm2290_dpu_caps = { > > .has_idle_pc = true, > > .max_linewidth = 2160, > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps sdm845_dpu_caps = { > > @@ -339,6 +343,8 @@ static const struct dpu_caps sdm845_dpu_caps = { > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > .max_hdeci_exp = MAX_HORZ_DECIMATION, > > .max_vdeci_exp = MAX_VERT_DECIMATION, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps sc7180_dpu_caps = { > > @@ -350,6 +356,8 @@ static const struct dpu_caps sc7180_dpu_caps = { > > .has_idle_pc = true, > > .max_linewidth = DEFAULT_DPU_OUTPUT_LINE_WIDTH, > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps sm6115_dpu_caps = { > > @@ -361,6 +369,8 @@ static const struct dpu_caps sm6115_dpu_caps = { > > .has_idle_pc = true, > > .max_linewidth = 2160, > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps sm8150_dpu_caps = { > > @@ -376,6 +386,8 @@ static const struct dpu_caps sm8150_dpu_caps = { > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > .max_hdeci_exp = MAX_HORZ_DECIMATION, > > .max_vdeci_exp = MAX_VERT_DECIMATION, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps sc8180x_dpu_caps = { > > @@ -391,6 +403,8 @@ static const struct dpu_caps sc8180x_dpu_caps = { > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > .max_hdeci_exp = MAX_HORZ_DECIMATION, > > .max_vdeci_exp = MAX_VERT_DECIMATION, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps sc8280xp_dpu_caps = { > > @@ -404,6 +418,8 @@ static const struct dpu_caps sc8280xp_dpu_caps = { > > .has_3d_merge = true, > > .max_linewidth = 5120, > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps sm8250_dpu_caps = { > > @@ -417,6 +433,8 @@ static const struct dpu_caps sm8250_dpu_caps = { > > .has_3d_merge = true, > > .max_linewidth = 900, > > .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, > > + .format_list = plane_formats_yuv, > > + .num_formats = ARRAY_SIZE(plane_formats_yuv), > > }; > > > > static const struct dpu_caps
Re: [Freedreno] [RFC PATCH v2 09/13] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check
On Thu, 25 May 2023 at 02:04, Abhinav Kumar wrote: > > > > On 5/24/2023 3:46 PM, Abhinav Kumar wrote: > > > > > > On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: > >> In preparation to virtualized planes support, move pstate->pipe > >> initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In > >> case of virtual planes the plane's pipe will not be known up to the > >> point of atomic_check() callback. > >> > >> Signed-off-by: Dmitry Baryshkov > >> --- > > > > Will legacy paths be broken with this? So lets say there is no > > atomic_check we will not have a valid sspp anymore. > > I think it should still work, even if goes through the modeset crtc, it > should still call drm_atomic_commit() internally which should have the > call to atomic_check, once you confirm this , i can ack this particular > change. Can you please describe the scenario you have in mind? If I got you correctly, you were asking about the non-commit IOCTLs. Because of the atomic helpers being used (e.g. drm_atomic_helper_set_config()), they will also result in a call to drm_atomic_commit(), which invokes drm_atomic_check_only(). -- With best wishes Dmitry
Re: [Freedreno] [PATCH v14 1/9] drm/display/dsc: Add flatness and initial scale value calculations
On 5/24/2023 12:05 PM, Marijn Suijten wrote: On 2023-05-24 10:45:14, Jessica Zhang wrote: Add helpers to calculate det_thresh_flatness and initial_scale_value as these calculations are defined within the DSC spec. Reviewed-by: Marijn Suijten Reviewed-by: Dmitry Baryshkov Signed-off-by: Jessica Zhang --- drivers/gpu/drm/display/drm_dsc_helper.c | 24 include/drm/display/drm_dsc_helper.h | 2 ++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c index fc187a8d8873..4efb6236d22c 100644 --- a/drivers/gpu/drm/display/drm_dsc_helper.c +++ b/drivers/gpu/drm/display/drm_dsc_helper.c @@ -1413,3 +1413,27 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg) return 0; } EXPORT_SYMBOL(drm_dsc_compute_rc_parameters); + +/** + * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config + * @dsc: Pointer to DRM DSC config struct + * + * Return: Calculated initial scale value Perhaps just drop Calculated from Return:? + */ +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc) +{ + return 8 * dsc->rc_model_size / (dsc->rc_model_size - dsc->initial_offset); +} +EXPORT_SYMBOL(drm_dsc_initial_scale_value); + +/** + * drm_dsc_flatness_det_thresh() - Calculate the flatness_det_thresh for the given DSC config You've written out the word ("flatness det thresh" and "initial scale value") entirely elsewhere, why the underscores in the doc comment here? Instead we should have the full meaning here (and in the Return: below), please correct me if I'm wrong but in VESA DSC v1.2a spec 6.8.5.1 Encoder Flatness Decision I think this variable means "flatness determination threshold"? If so, use that in the doc comment :) (and drop the leading "the", so just "Calculate flatness determination threshold for the given DSC config") + * @dsc: Pointer to DRM DSC config struct + * + * Return: Calculated flatness det thresh value Nit: perhaps we can just drop "calculated" here? Hi Marijn, Sure, I will make these changes if a v15 is necessary. In the future, can we try to group comments on wording/grammar/patch formatting with comments on the code itself? I really appreciate your feedback and help in improving the documentation around this feature, however I don't find it very productive to have revisions where the only changes are on (in my opinion) small wording details. Thanks, Jessica Zhang - Marijn + */ +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc) +{ + return 2 << (dsc->bits_per_component - 8); +} +EXPORT_SYMBOL(drm_dsc_flatness_det_thresh); diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h index fc2104415dcb..71789fb34e17 100644 --- a/include/drm/display/drm_dsc_helper.h +++ b/include/drm/display/drm_dsc_helper.h @@ -24,6 +24,8 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp, 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, enum drm_dsc_params_type type); int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc); +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc); #endif /* _DRM_DSC_HELPER_H_ */ -- 2.40.1
Re: [Freedreno] [RFC PATCH v2 11/13] drm/msm/dpu: add a field describing inline rotation to dpu_caps
On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: We need to know if the platform supports inline rotation on any of the SSPP blocks or not. Add this information to struct dpu_caps in a form of the boolean field has_inline_rot. So even for this one, will a helper to detect it from the list of sspps be better? We are now duplicating information from sspp to dpu caps for convenience and nothing wrong with it if the helper will get called like every atomic check . But looking at the next patch, it seems we use this information only once during dpu_plane_init(), so why not add a helper to find this out? Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 2d6944a9679a..33527ec7c938 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -489,6 +489,7 @@ static const struct dpu_caps sc7280_dpu_caps = { .ubwc_version = DPU_HW_UBWC_VER_30, .has_dim_layer = true, .has_idle_pc = true, + .has_inline_rot = true, .max_linewidth = 2400, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, .format_list = plane_formats_yuv, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index 4847aae78db2..cc64fb2e815f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -400,6 +400,7 @@ struct dpu_rotation_cfg { * @has_dim_layer dim layer feature status * @has_idle_pcindicate if idle power collapse feature is supported * @has_3d_merge indicate if 3D merge is supported + * @has_inline_rot indicate if inline rotation is supported * @max_linewidth max linewidth for sspp * @pixel_ram_size size of latency hiding and de-tiling buffer in bytes * @max_hdeci_exp max horizontal decimation supported (max is 2^value) @@ -416,6 +417,7 @@ struct dpu_caps { bool has_dim_layer; bool has_idle_pc; bool has_3d_merge; + bool has_inline_rot; /* SSPP limits */ u32 max_linewidth; u32 pixel_ram_size;
Re: [Freedreno] [RFC PATCH v2 10/13] drm/msm/dpu: add list of supported formats to the DPU caps
On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: As we are going to add virtual planes, add the list of supported formats to the hw catalog entry. It will be used to setup universal planes, with later selecting a pipe depending on whether the YUV format is used for the framebuffer. If your usage of format_list is going to be internal to dpu_plane.c, I can think of another idea for this change. This essentially translates to if (num_vig >= 1) If we can just have a small helper to detect that from the catalog can we use that instead of adding formats to the dpu caps? Signed-off-by: Dmitry Baryshkov --- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 26 +++ .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 4 +++ 2 files changed, 30 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 212d546b6c5d..2d6944a9679a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -315,6 +315,8 @@ static const struct dpu_caps msm8998_dpu_caps = { .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, .max_hdeci_exp = MAX_HORZ_DECIMATION, .max_vdeci_exp = MAX_VERT_DECIMATION, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps qcm2290_dpu_caps = { @@ -324,6 +326,8 @@ static const struct dpu_caps qcm2290_dpu_caps = { .has_idle_pc = true, .max_linewidth = 2160, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sdm845_dpu_caps = { @@ -339,6 +343,8 @@ static const struct dpu_caps sdm845_dpu_caps = { .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, .max_hdeci_exp = MAX_HORZ_DECIMATION, .max_vdeci_exp = MAX_VERT_DECIMATION, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sc7180_dpu_caps = { @@ -350,6 +356,8 @@ static const struct dpu_caps sc7180_dpu_caps = { .has_idle_pc = true, .max_linewidth = DEFAULT_DPU_OUTPUT_LINE_WIDTH, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sm6115_dpu_caps = { @@ -361,6 +369,8 @@ static const struct dpu_caps sm6115_dpu_caps = { .has_idle_pc = true, .max_linewidth = 2160, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sm8150_dpu_caps = { @@ -376,6 +386,8 @@ static const struct dpu_caps sm8150_dpu_caps = { .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, .max_hdeci_exp = MAX_HORZ_DECIMATION, .max_vdeci_exp = MAX_VERT_DECIMATION, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sc8180x_dpu_caps = { @@ -391,6 +403,8 @@ static const struct dpu_caps sc8180x_dpu_caps = { .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, .max_hdeci_exp = MAX_HORZ_DECIMATION, .max_vdeci_exp = MAX_VERT_DECIMATION, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sc8280xp_dpu_caps = { @@ -404,6 +418,8 @@ static const struct dpu_caps sc8280xp_dpu_caps = { .has_3d_merge = true, .max_linewidth = 5120, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sm8250_dpu_caps = { @@ -417,6 +433,8 @@ static const struct dpu_caps sm8250_dpu_caps = { .has_3d_merge = true, .max_linewidth = 900, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sm8350_dpu_caps = { @@ -430,6 +448,8 @@ static const struct dpu_caps sm8350_dpu_caps = { .has_3d_merge = true, .max_linewidth = 4096, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sm8450_dpu_caps = { @@ -443,6 +463,8 @@ static const struct dpu_caps sm8450_dpu_caps = { .has_3d_merge = true, .max_linewidth = 5120, .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE, + .format_list = plane_formats_yuv, + .num_formats = ARRAY_SIZE(plane_formats_yuv), }; static const struct dpu_caps sm8550_dpu_caps = { @@ -456,6 +478,8 @@ static const struct dpu_caps sm8550_dpu_caps = { .has_3d_merge = true,
Re: [Freedreno] [RFC PATCH v2 09/13] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check
On 5/24/2023 3:46 PM, Abhinav Kumar wrote: On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: In preparation to virtualized planes support, move pstate->pipe initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In case of virtual planes the plane's pipe will not be known up to the point of atomic_check() callback. Signed-off-by: Dmitry Baryshkov --- Will legacy paths be broken with this? So lets say there is no atomic_check we will not have a valid sspp anymore. I think it should still work, even if goes through the modeset crtc, it should still call drm_atomic_commit() internally which should have the call to atomic_check, once you confirm this , i can ack this particular change.
Re: [Freedreno] [RFC PATCH v2 09/13] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check
On 3/20/2023 6:18 PM, Dmitry Baryshkov wrote: In preparation to virtualized planes support, move pstate->pipe initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In case of virtual planes the plane's pipe will not be known up to the point of atomic_check() callback. Signed-off-by: Dmitry Baryshkov --- Will legacy paths be broken with this? So lets say there is no atomic_check we will not have a valid sspp anymore.
Re: [Freedreno] [PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper
On 5/24/2023 12:14 PM, Marijn Suijten wrote: On 2023-05-24 10:45:16, Jessica Zhang wrote: Add helper to get the integer value of drm_dsc_config.bits_per_pixel Reviewed-by: Marijn Suijten Reviewed-by: Dmitry Baryshkov Signed-off-by: Jessica Zhang Nit: if there comes a v15, perhaps this can be squashed with patch 1/9? I always thought they were separate because one extends the header while this extends the C file... but now both extend the C file with helpers. Hi Marijn, Sure, will squash this if there is a v15. --- drivers/gpu/drm/display/drm_dsc_helper.c | 13 + include/drm/display/drm_dsc_helper.h | 1 + 2 files changed, 14 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c index b31fe9849784..4424380c6cb6 100644 --- a/drivers/gpu/drm/display/drm_dsc_helper.c +++ b/drivers/gpu/drm/display/drm_dsc_helper.c @@ -1436,6 +1436,19 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg) } EXPORT_SYMBOL(drm_dsc_compute_rc_parameters); +/** + * drm_dsc_get_bpp_int() - Get integer bits per pixel value for the given DRM DSC config + * @vdsc_cfg: Pointer to DRM DSC config struct + * + * Return: Integer BPP value + */ +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg) +{ + WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf); You did not add linux/bug.h back, presumably because Dmitry added another use of WARN_ON_ONCE to this file in a previous series and it compiles fine as the definition trickles in via another header? Yep, this compiles fine without any error or warning. Thanks, Jessica Zhang - Marijn + return vdsc_cfg->bits_per_pixel >> 4; +} +EXPORT_SYMBOL(drm_dsc_get_bpp_int); + /** * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config * @dsc: Pointer to DRM DSC config struct diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h index f4e18e5d077a..913aa2071232 100644 --- a/include/drm/display/drm_dsc_helper.h +++ b/include/drm/display/drm_dsc_helper.h @@ -27,6 +27,7 @@ int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc); u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc); +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg); #endif /* _DRM_DSC_HELPER_H_ */ -- 2.40.1
Re: [Freedreno] [PATCH v13 02/10] drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X
Title: DSC On 2023-05-22 17:00:31, Kuogee Hsieh wrote: > From: Abhinav Kumar > > Some platforms have DSC blocks which have not been declared in the catalog. > Complete DSC 1.1 support for all platforms by adding the missing blocks to > MSM8998 and SC8180X. > > Changes in v9: > -- add MSM8998 and SC8180x to commit title > > Changes in v10: > -- fix grammar at commit text > > Changes in v12: > -- fix "titil" with "title" at changes in v9 > > Signed-off-by: Abhinav Kumar > Reviewed-by: Dmitry Baryshkov > Reviewed-by: Marijn Suijten > --- > drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +++ > drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +++ > 2 files changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > index c0dd477..521cfd5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h > @@ -126,6 +126,11 @@ static const struct dpu_pingpong_cfg msm8998_pp[] = { > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)), > }; > > +static const struct dpu_dsc_cfg msm8998_dsc[] = { > + DSC_BLK("dsc_0", DSC_0, 0x8, 0), > + DSC_BLK("dsc_1", DSC_1, 0x80400, 0), > +}; > + > static const struct dpu_dspp_cfg msm8998_dspp[] = { > DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK, >_dspp_sblk), > @@ -199,6 +204,8 @@ const struct dpu_mdss_cfg dpu_msm8998_cfg = { > .dspp = msm8998_dspp, > .pingpong_count = ARRAY_SIZE(msm8998_pp), > .pingpong = msm8998_pp, > + .dsc_count = ARRAY_SIZE(msm8998_dsc), > + .dsc = msm8998_dsc, > .intf_count = ARRAY_SIZE(msm8998_intf), > .intf = msm8998_intf, > .vbif_count = ARRAY_SIZE(msm8998_vbif), > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > index e8057a1..fec1665 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h > @@ -142,6 +142,15 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] > = { > MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200), > }; > > +static const struct dpu_dsc_cfg sc8180x_dsc[] = { > + DSC_BLK("dsc_0", DSC_0, 0x8, BIT(DPU_DSC_OUTPUT_CTRL)), > + DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)), > + DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)), > + DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)), > + DSC_BLK("dsc_4", DSC_4, 0x81000, BIT(DPU_DSC_OUTPUT_CTRL)), > + DSC_BLK("dsc_5", DSC_5, 0x81400, BIT(DPU_DSC_OUTPUT_CTRL)), > +}; > + > static const struct dpu_intf_cfg sc8180x_intf[] = { > INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, > MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK, > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24), > @@ -206,6 +215,8 @@ const struct dpu_mdss_cfg dpu_sc8180x_cfg = { > .mixer = sc8180x_lm, > .pingpong_count = ARRAY_SIZE(sc8180x_pp), > .pingpong = sc8180x_pp, > + .dsc_count = ARRAY_SIZE(sc8180x_dsc), > + .dsc = sc8180x_dsc, > .merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d), > .merge_3d = sc8180x_merge_3d, > .intf_count = ARRAY_SIZE(sc8180x_intf), > -- > 2.7.4 >
Re: [Freedreno] [PATCH v13 01/10] drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register
On 2023-05-22 17:00:30, Kuogee Hsieh wrote: > The DSC CTL_FLUSH register should be programmed with the 22th bit Sorry for botching this in v12 review, there's no DSC CTL_FLUSH register. Drop DSC from "The DSC CTL_FLUSH register". > (DSC_IDX) to flush the DSC hardware blocks, not the literal value of > 22 (which corresponds to flushing VIG1, VIG2 and RGB1 instead). > > Changes in V12: > -- split this patch out of "separate DSC flush update out of interface" > > Changes in V13: > -- rewording the commit text > > Fixes: 77f6da90487c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl") > Signed-off-by: Kuogee Hsieh Reviewed-by: Marijn Suijten > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > index 4f7cfa9..69d0ea2 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > @@ -525,7 +525,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx, > DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE, > BIT(cfg->merge_3d - MERGE_3D_0)); > if (cfg->dsc) { > - DPU_REG_WRITE(>hw, CTL_FLUSH, DSC_IDX); > + DPU_REG_WRITE(>hw, CTL_FLUSH, BIT(DSC_IDX)); > DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc); > } > } > -- > 2.7.4 >
Re: [Freedreno] [PATCH v13 08/10] drm/msm/dpu: separate DSC flush update out of interface
On 2023-05-22 17:00:37, Kuogee Hsieh wrote: > Currently DSC flushing happens during interface configuration at > dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from > dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1() > to handle both per-DSC engine and DSC flush bits at same time to make it > consistent with the location of flush programming of other DPU sub-blocks. > > Changes in v10: > -- rewording commit text > -- pass ctl directly instead of dpu_enc to dsc_pipe_cfg() > -- ctx->pending_dsc_flush_mask = 0; > > Changes in v11: > -- add Fixes tag > -- capitalize MERGE_3D, DSPP and DSC at struct dpu_hw_ctl_ops{} But MERGE_3D and DSPP documentation entries were removed some time later, though this is not reflected anywhere in the changelog. > > Changes in v12: > -- move dsc parameter to next line at dpu_encoder_dsc_pipe_cfg() > > Signed-off-by: Kuogee Hsieh > Reviewed-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 -- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 23 +-- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 11 +++ > 3 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index ffa6f04..7fca09e 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config > *dsc, > return DIV_ROUND_UP(total_pixels, dsc->slice_width); > } > > -static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc, > +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl, > + struct dpu_hw_dsc *hw_dsc, >struct dpu_hw_pingpong *hw_pp, >struct drm_dsc_config *dsc, >u32 common_mode, > @@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc > *hw_dsc, > > if (hw_pp->ops.enable_dsc) > hw_pp->ops.enable_dsc(hw_pp); > + > + if (ctl->ops.update_pending_flush_dsc) > + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx); > } > > static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, > @@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct > dpu_encoder_virt *dpu_enc, > { > /* coding only for 2LM, 2enc, 1 dsc config */ > struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; > + struct dpu_hw_ctl *ctl = enc_master->hw_ctl; > struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; > struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC]; > int this_frame_slices; > @@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct > dpu_encoder_virt *dpu_enc, > initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); > > for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) > - dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, > dsc_common_mode, initial_lines); > + dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], > + dsc, dsc_common_mode, initial_lines); > } > > void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > index 64c21e0..ad6983e 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > @@ -103,6 +103,7 @@ static inline void dpu_hw_ctl_clear_pending_flush(struct > dpu_hw_ctl *ctx) > ctx->pending_intf_flush_mask = 0; > ctx->pending_wb_flush_mask = 0; > ctx->pending_merge_3d_flush_mask = 0; > + ctx->pending_dsc_flush_mask = 0; > > memset(ctx->pending_dspp_flush_mask, 0, > sizeof(ctx->pending_dspp_flush_mask)); > @@ -142,6 +143,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct > dpu_hw_ctl *ctx) > CTL_DSPP_n_FLUSH(dspp - DSPP_0), > ctx->pending_dspp_flush_mask[dspp - DSPP_0]); > } > + > + if (ctx->pending_flush_mask & BIT(DSC_IDX)) > + DPU_REG_WRITE(>hw, CTL_DSC_FLUSH, > + ctx->pending_dsc_flush_mask); > + > DPU_REG_WRITE(>hw, CTL_FLUSH, ctx->pending_flush_mask); > } > > @@ -288,6 +294,13 @@ static void > dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx, > ctx->pending_flush_mask |= BIT(MERGE_3D_IDX); > } > > +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx, > +enum dpu_dsc dsc_num) > +{ > + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0); > + ctx->pending_flush_mask |= BIT(DSC_IDX); > +} > + > static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl
Re: [Freedreno] [PATCH v13 10/10] drm/msm/dpu: tear down DSC data path when DSC disabled
Title: Tear down DSC datapath on encoder cleanup (from v10 review, this is v13...) On 2023-05-22 17:00:39, Kuogee Hsieh wrote: - Marijn
Re: [Freedreno] [PATCH v13 07/10] drm/msm/dpu: always clear every individual pending flush mask
On 2023-05-22 17:00:36, Kuogee Hsieh wrote: > There are two tiers of pending flush control, top levle and levle -> level > individual hardware block. Currently only the top level of > flush mask is reset to 0 but the individual pending flush masks > of particular hardware blocks are left at their previous values, > eventually accumulating all possible bit values and typically > flushing more than necessary. > Reset all individual hardware blocks flush masks to 0 to avoid block, drop -s, because masks is plural. > individual hardware block be triggered accidentally. be = from being triggered = flushed? (You just said "individual hardware block", it would be okay to refer to that with just "Reset all individual hardware block flush masks to 0 to avoid accidentally flushing them.") > > Changes in V13: > -- rewording commi ttext commit text > -- add an empty space line as suggested > > Signed-off-by: Kuogee Hsieh > Reviewed-by: Dmitry Baryshkov > Reviewed-by: Marijn Suijten So no fixes tag? - Marijn > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > index 69d0ea2..64c21e0 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > @@ -100,6 +100,9 @@ static inline void dpu_hw_ctl_clear_pending_flush(struct > dpu_hw_ctl *ctx) > trace_dpu_hw_ctl_clear_pending_flush(ctx->pending_flush_mask, >dpu_hw_ctl_get_flush_register(ctx)); > ctx->pending_flush_mask = 0x0; > + ctx->pending_intf_flush_mask = 0; > + ctx->pending_wb_flush_mask = 0; > + ctx->pending_merge_3d_flush_mask = 0; > > memset(ctx->pending_dspp_flush_mask, 0, > sizeof(ctx->pending_dspp_flush_mask)); > -- > 2.7.4 >
Re: [Freedreno] [PATCH v3 1/9] drm/msm/dpu: fix SSPP register definitions
On 5/18/2023 3:22 PM, Dmitry Baryshkov wrote: Reorder SSPP register definitions to sort them in the ascending order. Move register bitfields after the register definitions. Signed-off-by: Dmitry Baryshkov --- Reviewed-by: Jeykumar Sankaran drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 64 ++--- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c index 6b68ec5c7a5a..08098880b7d5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -26,45 +26,18 @@ #define SSPP_SRC_FORMAT0x30 #define SSPP_SRC_UNPACK_PATTERN0x34 #define SSPP_SRC_OP_MODE 0x38 - -/* SSPP_MULTIRECT*/ -#define SSPP_SRC_SIZE_REC1 0x16C -#define SSPP_SRC_XY_REC1 0x168 -#define SSPP_OUT_SIZE_REC1 0x160 -#define SSPP_OUT_XY_REC1 0x164 -#define SSPP_SRC_FORMAT_REC1 0x174 -#define SSPP_SRC_UNPACK_PATTERN_REC1 0x178 -#define SSPP_SRC_OP_MODE_REC1 0x17C -#define SSPP_MULTIRECT_OPMODE 0x170 -#define SSPP_SRC_CONSTANT_COLOR_REC1 0x180 -#define SSPP_EXCL_REC_SIZE_REC10x184 -#define SSPP_EXCL_REC_XY_REC1 0x188 - -#define MDSS_MDP_OP_DEINTERLACEBIT(22) -#define MDSS_MDP_OP_DEINTERLACE_ODDBIT(23) -#define MDSS_MDP_OP_IGC_ROM_1 BIT(18) -#define MDSS_MDP_OP_IGC_ROM_0 BIT(17) -#define MDSS_MDP_OP_IGC_EN BIT(16) -#define MDSS_MDP_OP_FLIP_UDBIT(14) -#define MDSS_MDP_OP_FLIP_LRBIT(13) -#define MDSS_MDP_OP_BWC_EN BIT(0) -#define MDSS_MDP_OP_PE_OVERRIDEBIT(31) -#define MDSS_MDP_OP_BWC_LOSSLESS (0 << 1) -#define MDSS_MDP_OP_BWC_Q_HIGH (1 << 1) -#define MDSS_MDP_OP_BWC_Q_MED (2 << 1) - #define SSPP_SRC_CONSTANT_COLOR0x3c #define SSPP_EXCL_REC_CTL 0x40 #define SSPP_UBWC_STATIC_CTRL 0x44 -#define SSPP_FETCH_CONFIG 0x048 +#define SSPP_FETCH_CONFIG 0x48 #define SSPP_DANGER_LUT0x60 #define SSPP_SAFE_LUT 0x64 #define SSPP_CREQ_LUT 0x68 #define SSPP_QOS_CTRL 0x6C -#define SSPP_DECIMATION_CONFIG 0xB4 #define SSPP_SRC_ADDR_SW_STATUS0x70 #define SSPP_CREQ_LUT_00x74 #define SSPP_CREQ_LUT_10x78 +#define SSPP_DECIMATION_CONFIG 0xB4 #define SSPP_SW_PIX_EXT_C0_LR 0x100 #define SSPP_SW_PIX_EXT_C0_TB 0x104 #define SSPP_SW_PIX_EXT_C0_REQ_PIXELS 0x108 @@ -81,11 +54,33 @@ #define SSPP_TRAFFIC_SHAPER_PREFILL0x150 #define SSPP_TRAFFIC_SHAPER_REC1_PREFILL 0x154 #define SSPP_TRAFFIC_SHAPER_REC1 0x158 +#define SSPP_OUT_SIZE_REC1 0x160 +#define SSPP_OUT_XY_REC1 0x164 +#define SSPP_SRC_XY_REC1 0x168 +#define SSPP_SRC_SIZE_REC1 0x16C +#define SSPP_MULTIRECT_OPMODE 0x170 +#define SSPP_SRC_FORMAT_REC1 0x174 +#define SSPP_SRC_UNPACK_PATTERN_REC1 0x178 +#define SSPP_SRC_OP_MODE_REC1 0x17C +#define SSPP_SRC_CONSTANT_COLOR_REC1 0x180 +#define SSPP_EXCL_REC_SIZE_REC10x184 +#define SSPP_EXCL_REC_XY_REC1 0x188 #define SSPP_EXCL_REC_SIZE 0x1B4 #define SSPP_EXCL_REC_XY 0x1B8 -#define SSPP_VIG_OP_MODE 0x0 -#define SSPP_VIG_CSC_10_OP_MODE0x0 -#define SSPP_TRAFFIC_SHAPER_BPC_MAX0xFF + +/* SSPP_SRC_OP_MODE & OP_MODE_REC1 */ +#define MDSS_MDP_OP_DEINTERLACEBIT(22) +#define MDSS_MDP_OP_DEINTERLACE_ODDBIT(23) +#define MDSS_MDP_OP_IGC_ROM_1 BIT(18) +#define MDSS_MDP_OP_IGC_ROM_0 BIT(17) +#define MDSS_MDP_OP_IGC_EN BIT(16) +#define MDSS_MDP_OP_FLIP_UDBIT(14) +#define MDSS_MDP_OP_FLIP_LRBIT(13) +#define MDSS_MDP_OP_BWC_EN BIT(0) +#define MDSS_MDP_OP_PE_OVERRIDEBIT(31) +#define MDSS_MDP_OP_BWC_LOSSLESS (0 << 1) +#define MDSS_MDP_OP_BWC_Q_HIGH (1 << 1) +#define MDSS_MDP_OP_BWC_Q_MED (2 << 1) /* SSPP_QOS_CTRL */ #define SSPP_QOS_CTRL_VBLANK_ENBIT(16) @@ -96,6 +91,7 @@ #define SSPP_QOS_CTRL_CREQ_VBLANK_OFF 20 /* DPU_SSPP_SCALER_QSEED2 */ +#define SSPP_VIG_OP_MODE 0x0 #define SCALE_CONFIG 0x04 #define COMP0_3_PHASE_STEP_X 0x10 #define COMP0_3_PHASE_STEP_Y 0x14 @@ -107,6 +103,9 @@ #define COMP1_2_INIT_PHASE_Y 0x2C #define VIG_0_QSEED2_SHARP 0x30 +/*
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
On 5/24/2023 2:48 AM, Marijn Suijten wrote: On 2023-05-23 13:01:13, Abhinav Kumar wrote: On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote: Drop SSPP-specifig debugfs register dumps in favour of using debugfs/dri/0/kms or devcoredump. I did see another series which removes src_blk from the catalog (I am yet to review that one) . Lets assume that one is fine and this change will be going on top of that one right? It replaces src_blk with directly accessing the blk (non-sub-block) directly, because they were overlapping anyway. The concern I have with this change is that although I do agree that we should be in favor of using debugfs/dri/0/kms ( i have used it a few times and it works pretty well ), devcoredump does not have the support to dump sub-blocks . Something which we should add with priority because even with DSC blocks with the separation of enc/ctl blocks we need that like I wrote in one of the responses. So the "len" of the blocks having sub-blocks will be ignored in favor of the len of the sub-blocks. The sub-blocks are not always contiguous with their parent block, are they? It's probably better to print the sub-blocks separately with Yes, not contiguous otherwise we could have just had them in one big range. clear headers anyway rather than dumping the range parent_blk_base to max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...). - Marijn When I meant sub-block support to devcoredump, this is how I visualize them to be printed =SSPP xxx === =SSPP_CSC ===(for SSPP_xxx) =SSPP_QSEED =(for SSPP_xxx) etc OR for DSC DSC_xxx == DSC_CTL == (for DSC_xxx) DSC_ENC ===(for DSC_xxx) This is clear enough headers. If we remove this without adding that support first, its a loss of debug functionality. Can we retain these blocks and remove dpu_debugfs_create_regset32 in a different way?
Re: [Freedreno] [PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper
On 2023-05-24 10:45:16, Jessica Zhang wrote: > Add helper to get the integer value of drm_dsc_config.bits_per_pixel > > Reviewed-by: Marijn Suijten > Reviewed-by: Dmitry Baryshkov > Signed-off-by: Jessica Zhang Nit: if there comes a v15, perhaps this can be squashed with patch 1/9? I always thought they were separate because one extends the header while this extends the C file... but now both extend the C file with helpers. > --- > drivers/gpu/drm/display/drm_dsc_helper.c | 13 + > include/drm/display/drm_dsc_helper.h | 1 + > 2 files changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c > b/drivers/gpu/drm/display/drm_dsc_helper.c > index b31fe9849784..4424380c6cb6 100644 > --- a/drivers/gpu/drm/display/drm_dsc_helper.c > +++ b/drivers/gpu/drm/display/drm_dsc_helper.c > @@ -1436,6 +1436,19 @@ int drm_dsc_compute_rc_parameters(struct > drm_dsc_config *vdsc_cfg) > } > EXPORT_SYMBOL(drm_dsc_compute_rc_parameters); > > +/** > + * drm_dsc_get_bpp_int() - Get integer bits per pixel value for the given > DRM DSC config > + * @vdsc_cfg: Pointer to DRM DSC config struct > + * > + * Return: Integer BPP value > + */ > +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg) > +{ > + WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf); You did not add linux/bug.h back, presumably because Dmitry added another use of WARN_ON_ONCE to this file in a previous series and it compiles fine as the definition trickles in via another header? - Marijn > + return vdsc_cfg->bits_per_pixel >> 4; > +} > +EXPORT_SYMBOL(drm_dsc_get_bpp_int); > + > /** > * drm_dsc_initial_scale_value() - Calculate the initial scale value for the > given DSC config > * @dsc: Pointer to DRM DSC config struct > diff --git a/include/drm/display/drm_dsc_helper.h > b/include/drm/display/drm_dsc_helper.h > index f4e18e5d077a..913aa2071232 100644 > --- a/include/drm/display/drm_dsc_helper.h > +++ b/include/drm/display/drm_dsc_helper.h > @@ -27,6 +27,7 @@ int drm_dsc_setup_rc_params(struct drm_dsc_config > *vdsc_cfg, enum drm_dsc_params > int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); > u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc); > u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc); > +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg); > > #endif /* _DRM_DSC_HELPER_H_ */ > > > -- > 2.40.1 >
Re: [Freedreno] [PATCH v14 0/9] Introduce MSM-specific DSC helpers
On 2023-05-24 10:45:13, Jessica Zhang wrote: > There are some overlap in calculations for MSM-specific DSC variables > between DP and DSI. In addition, the calculations for initial_scale_value > and det_thresh_flatness that are defined within the DSC 1.2 specifications, > but aren't yet included in drm_dsc_helper.c. > > This series moves these calculations to a shared msm_dsc_helper.c file and > defines drm_dsc_helper methods for initial_scale_value and > det_thresh_flatness. > > Note: For now, the MSM specific helper methods are only called for the DSI > path, but will called for DP once DSC 1.2 support for DP has been added. > > Depends on: "drm/i915: move DSC RC tables to drm_dsc_helper.c" [1] > > [1] https://patchwork.freedesktop.org/series/114472/ > > --- > Changes in v14: > - Added kernel docs and made DRM DSC helper functions (Jani) They were already helper functions: I think you meant to write that you have moved from from inlined header functions to exported symbols in the .c file? - Marijn > - Link to v13: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v13-0-d7581e7be...@quicinc.com > > Changes in v13: > - Reworded comment doc for msm_dsc_get_slices_per_intf() > - Changed intf_width to u32 > - msm_dsc_calculate_slices_per_intf -> msm_dsc_get_slices_per_intf > - Link to v12: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v12-0-9cdb7401f...@quicinc.com > > Changes in v12: > - Reworded summary comment for msm_dsc_helper.h > - msm_dsc_get_slices_per_intf -> msm_dsc_calculate_slices_per_intf > - Corrected total_bytes_per_intf math in dsc_update_dsc_timing > - Rebased on top of latest version of "drm/i915: move DSC RC tables to > drm_dsc_helper.c" > - Link to v11: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v11-0-30270e1ee...@quicinc.com > > Changes in v11: > - Fixed mismatched return types > - Moved MSM DSC helpers summary comment to under copyright > - Moved msm_dsc_get_bpp_int() to drm_dsc_helper.h > - Reworded MSM DSC helper comment docs for clarity > - Added const keyword where applicable > - Renamed msm_dsc_get_slice_per_intf to msm_dsc_get_slices_per_intf > - Removed msm_dsc_get_slice_per_intf() > - Reworded commit message for "drm/msm/dsi: update hdisplay calculation > for dsi_timing_setup" > - Link to v10: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v10-0-4cb21168c...@quicinc.com > > Changes in v10: > - Removed msm_dsc_get_bytes_per_slice helper > - Inlined msm_dsc_get_bytes_per_intf > - Refactored drm_dsc_set_initial_scale_value() to be a pure function > - Renamed DRM DSC initial_scale and flatness_det_thresh helpers > - Removed msm_dsc_helpers.o from Makefile > - Link to v9: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v9-0-87daeaec2...@quicinc.com > > Changes in v9: > - Fixed incorrect math for msm_dsc_get_bytes_per_line() > - Link to v8: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v8-0-2c9b2bb12...@quicinc.com > > Changes in v8: > - *_bytes_per_soft_slice --> *_bytes_per_slice > - Fixed comment doc formatting for MSM DSC helpers > - Use slice_chunk_size in msm_dsc_get_bytes_per_line calculation > - Reworded "drm/msm/dpu: Use DRM DSC helper for det_thresh_flatness" > commit title for clarity > - Picked up "Reviewed-by" tags > - Added duplicate Signed-off-by tag to "drm/display/dsc: Add flatness > and initial scale value calculations" to reflect patch history > - Link to v7: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v7-0-df48a2c54...@quicinc.com > > Changes in v7: > - Renamed msm_dsc_get_pclk_per_intf to msm_dsc_get_bytes_per_line > - Removed duplicate msm_dsc_get_dce_bytes_per_line > - Reworded commit message for "drm/msm/dpu: Use DRM DSC helper for > det_thresh_flatness" > - Dropped slice_per_pkt change (it will be included in the later > "Add DSC v1.2 Support for DSI" series) > - Picked up "drm/display/dsc: Add flatness and initial scale value > calculations" and "drm/display/dsc: add helper to set semi-const > parameters", which were dropped from "drm/i915: move DSC RC tables to > drm_dsc_helper.c" series > - Picked up "Reviewed-by" tags > - Removed changelog in individual patches > - Link to v6: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v6-0-cb7f59f0f...@quicinc.com > > Changes in v6: > - Documented return values for MSM DSC helpers > - Fixed dependency issue in msm_dsc_helper.c > - Link to v5: > https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v5-0-0108401d7...@quicinc.com > > Changes in v5: > - Added extra line at end of msm_dsc_helper.h > - Simplified msm_dsc_get_bytes_per_soft_slice() math > - Simplified and inlined msm_dsc_get_pclk_per_intf() math > - "Fix calculations pkt_per_line" --> "... Fix calculation for pkt_per_line" > - Split dsc->slice_width check into a separate patch > - Picked up Dmitry's msm/dsi patch ("drm/msm/dsi: use new helpers for > DSC setup") > - Removed unused headers in MSM DSC helper files > - Picked up Reviewed-by
Re: [Freedreno] [PATCH v14 1/9] drm/display/dsc: Add flatness and initial scale value calculations
On 2023-05-24 10:45:14, Jessica Zhang wrote: > Add helpers to calculate det_thresh_flatness and initial_scale_value as > these calculations are defined within the DSC spec. > > Reviewed-by: Marijn Suijten > Reviewed-by: Dmitry Baryshkov > Signed-off-by: Jessica Zhang > --- > drivers/gpu/drm/display/drm_dsc_helper.c | 24 > include/drm/display/drm_dsc_helper.h | 2 ++ > 2 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c > b/drivers/gpu/drm/display/drm_dsc_helper.c > index fc187a8d8873..4efb6236d22c 100644 > --- a/drivers/gpu/drm/display/drm_dsc_helper.c > +++ b/drivers/gpu/drm/display/drm_dsc_helper.c > @@ -1413,3 +1413,27 @@ int drm_dsc_compute_rc_parameters(struct > drm_dsc_config *vdsc_cfg) > return 0; > } > EXPORT_SYMBOL(drm_dsc_compute_rc_parameters); > + > +/** > + * drm_dsc_initial_scale_value() - Calculate the initial scale value for the > given DSC config > + * @dsc: Pointer to DRM DSC config struct > + * > + * Return: Calculated initial scale value Perhaps just drop Calculated from Return:? > + */ > +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc) > +{ > + return 8 * dsc->rc_model_size / (dsc->rc_model_size - > dsc->initial_offset); > +} > +EXPORT_SYMBOL(drm_dsc_initial_scale_value); > + > +/** > + * drm_dsc_flatness_det_thresh() - Calculate the flatness_det_thresh for the > given DSC config You've written out the word ("flatness det thresh" and "initial scale value") entirely elsewhere, why the underscores in the doc comment here? Instead we should have the full meaning here (and in the Return: below), please correct me if I'm wrong but in VESA DSC v1.2a spec 6.8.5.1 Encoder Flatness Decision I think this variable means "flatness determination threshold"? If so, use that in the doc comment :) (and drop the leading "the", so just "Calculate flatness determination threshold for the given DSC config") > + * @dsc: Pointer to DRM DSC config struct > + * > + * Return: Calculated flatness det thresh value Nit: perhaps we can just drop "calculated" here? - Marijn > + */ > +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc) > +{ > + return 2 << (dsc->bits_per_component - 8); > +} > +EXPORT_SYMBOL(drm_dsc_flatness_det_thresh); > diff --git a/include/drm/display/drm_dsc_helper.h > b/include/drm/display/drm_dsc_helper.h > index fc2104415dcb..71789fb34e17 100644 > --- a/include/drm/display/drm_dsc_helper.h > +++ b/include/drm/display/drm_dsc_helper.h > @@ -24,6 +24,8 @@ void drm_dsc_pps_payload_pack(struct > drm_dsc_picture_parameter_set *pps_sdp, > 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, enum > drm_dsc_params_type type); > int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); > +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc); > +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc); > > #endif /* _DRM_DSC_HELPER_H_ */ > > > -- > 2.40.1 >
[Freedreno] [PATCH v14 1/9] drm/display/dsc: Add flatness and initial scale value calculations
Add helpers to calculate det_thresh_flatness and initial_scale_value as these calculations are defined within the DSC spec. Reviewed-by: Marijn Suijten Reviewed-by: Dmitry Baryshkov Signed-off-by: Jessica Zhang --- drivers/gpu/drm/display/drm_dsc_helper.c | 24 include/drm/display/drm_dsc_helper.h | 2 ++ 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c index fc187a8d8873..4efb6236d22c 100644 --- a/drivers/gpu/drm/display/drm_dsc_helper.c +++ b/drivers/gpu/drm/display/drm_dsc_helper.c @@ -1413,3 +1413,27 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg) return 0; } EXPORT_SYMBOL(drm_dsc_compute_rc_parameters); + +/** + * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config + * @dsc: Pointer to DRM DSC config struct + * + * Return: Calculated initial scale value + */ +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc) +{ + return 8 * dsc->rc_model_size / (dsc->rc_model_size - dsc->initial_offset); +} +EXPORT_SYMBOL(drm_dsc_initial_scale_value); + +/** + * drm_dsc_flatness_det_thresh() - Calculate the flatness_det_thresh for the given DSC config + * @dsc: Pointer to DRM DSC config struct + * + * Return: Calculated flatness det thresh value + */ +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc) +{ + return 2 << (dsc->bits_per_component - 8); +} +EXPORT_SYMBOL(drm_dsc_flatness_det_thresh); diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h index fc2104415dcb..71789fb34e17 100644 --- a/include/drm/display/drm_dsc_helper.h +++ b/include/drm/display/drm_dsc_helper.h @@ -24,6 +24,8 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp, 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, enum drm_dsc_params_type type); int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc); +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc); #endif /* _DRM_DSC_HELPER_H_ */ -- 2.40.1
[Freedreno] [PATCH v14 2/9] drm/display/dsc: add helper to set semi-const parameters
From: Dmitry Baryshkov Add a helper setting config values which are typically constant across operating modes (table E-4 of the standard) and mux_word_size (which is a const according to 3.5.2). Signed-off-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten Signed-off-by: Jessica Zhang --- drivers/gpu/drm/display/drm_dsc_helper.c | 22 ++ include/drm/display/drm_dsc_helper.h | 1 + 2 files changed, 23 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c index 4efb6236d22c..b31fe9849784 100644 --- a/drivers/gpu/drm/display/drm_dsc_helper.c +++ b/drivers/gpu/drm/display/drm_dsc_helper.c @@ -270,6 +270,28 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload, } EXPORT_SYMBOL(drm_dsc_pps_payload_pack); +/** + * drm_dsc_set_const_params() - Set DSC parameters considered typically + * constant across operation modes + * + * @vdsc_cfg: + * DSC Configuration data partially filled by driver + */ +void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg) +{ + if (!vdsc_cfg->rc_model_size) + vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST; + vdsc_cfg->rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST; + vdsc_cfg->rc_tgt_offset_high = DSC_RC_TGT_OFFSET_HI_CONST; + vdsc_cfg->rc_tgt_offset_low = DSC_RC_TGT_OFFSET_LO_CONST; + + if (vdsc_cfg->bits_per_component <= 10) + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC; + else + vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC; +} +EXPORT_SYMBOL(drm_dsc_set_const_params); + /* From DSC_v1.11 spec, rc_parameter_Set syntax element typically constant */ static const u16 drm_dsc_rc_buf_thresh[] = { 896, 1792, 2688, 3584, 4480, 5376, 6272, 6720, 7168, 7616, diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h index 71789fb34e17..f4e18e5d077a 100644 --- a/include/drm/display/drm_dsc_helper.h +++ b/include/drm/display/drm_dsc_helper.h @@ -21,6 +21,7 @@ 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_const_params(struct drm_dsc_config *vdsc_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, enum drm_dsc_params_type type); int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); -- 2.40.1
[Freedreno] [PATCH v14 4/9] drm/msm/dsi: use DRM DSC helpers for DSC setup
From: Dmitry Baryshkov Use new DRM DSC helpers to setup DSI DSC configuration. The initial_scale_value needs to be adjusted according to the standard, but this is a separate change. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar Reviewed-by: Marijn Suijten Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/dsi/dsi_host.c | 61 +- 1 file changed, 8 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 961689a255c4..74d38f90398a 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1731,28 +1731,9 @@ static int dsi_host_parse_lane_data(struct msm_dsi_host *msm_host, return -EINVAL; } -static u32 dsi_dsc_rc_buf_thresh[DSC_NUM_BUF_RANGES - 1] = { - 0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62, - 0x69, 0x70, 0x77, 0x79, 0x7b, 0x7d, 0x7e -}; - -/* only 8bpc, 8bpp added */ -static char min_qp[DSC_NUM_BUF_RANGES] = { - 0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 7, 13 -}; - -static char max_qp[DSC_NUM_BUF_RANGES] = { - 4, 4, 5, 6, 7, 7, 7, 8, 9, 10, 11, 12, 13, 13, 15 -}; - -static char bpg_offset[DSC_NUM_BUF_RANGES] = { - 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12 -}; - static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc) { - int i; - u16 bpp = dsc->bits_per_pixel >> 4; + int ret; if (dsc->bits_per_pixel & 0xf) { DRM_DEV_ERROR(_host->pdev->dev, "DSI does not support fractional bits_per_pixel\n"); @@ -1764,49 +1745,23 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc return -EOPNOTSUPP; } - dsc->rc_model_size = 8192; - dsc->first_line_bpg_offset = 12; - dsc->rc_edge_factor = 6; - dsc->rc_tgt_offset_high = 3; - dsc->rc_tgt_offset_low = 3; dsc->simple_422 = 0; dsc->convert_rgb = 1; dsc->vbr_enable = 0; - /* handle only bpp = bpc = 8 */ - for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++) - dsc->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i]; + drm_dsc_set_const_params(dsc); + drm_dsc_set_rc_buf_thresh(dsc); - for (i = 0; i < DSC_NUM_BUF_RANGES; i++) { - dsc->rc_range_params[i].range_min_qp = min_qp[i]; - dsc->rc_range_params[i].range_max_qp = max_qp[i]; - /* -* Range BPG Offset contains two's-complement signed values that fill -* 8 bits, yet the registers and DCS PPS field are only 6 bits wide. -*/ - dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i] & DSC_RANGE_BPG_OFFSET_MASK; + /* handle only bpp = bpc = 8, pre-SCR panels */ + ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR); + if (ret) { + DRM_DEV_ERROR(_host->pdev->dev, "could not find DSC RC parameters\n"); + return ret; } - dsc->initial_offset = 6144; /* Not bpp 12 */ - if (bpp != 8) - dsc->initial_offset = 2048; /* bpp = 12 */ - - if (dsc->bits_per_component <= 10) - dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC; - else - dsc->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC; - - dsc->initial_xmit_delay = 512; dsc->initial_scale_value = 32; - dsc->first_line_bpg_offset = 12; dsc->line_buf_depth = dsc->bits_per_component + 1; - /* bpc 8 */ - dsc->flatness_min_qp = 3; - dsc->flatness_max_qp = 12; - dsc->rc_quant_incr_limit0 = 11; - dsc->rc_quant_incr_limit1 = 11; - return drm_dsc_compute_rc_parameters(dsc); } -- 2.40.1
[Freedreno] [PATCH v14 6/9] drm/msm/dpu: Use fixed DRM DSC helper for det_thresh_flatness
The current dpu_hw_dsc calculation for det_thresh_flatness does not match the downstream calculation or the DSC spec. Use the DRM DSC helper for det_thresh_flatness to match downstream implementation and the DSC spec. Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC") Reviewed-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c index 4e1396575e6a..3cad6a80af97 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c @@ -3,6 +3,8 @@ * Copyright (c) 2020-2022, Linaro Limited */ +#include + #include "dpu_kms.h" #include "dpu_hw_catalog.h" #include "dpu_hwio.h" @@ -102,7 +104,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc, data |= dsc->final_offset; DPU_REG_WRITE(c, DSC_DSC_OFFSET, data); - det_thresh_flatness = 7 + 2 * (dsc->bits_per_component - 8); + det_thresh_flatness = drm_dsc_flatness_det_thresh(dsc); data = det_thresh_flatness << 10; data |= dsc->flatness_max_qp << 5; data |= dsc->flatness_min_qp; -- 2.40.1
[Freedreno] [PATCH v14 8/9] drm/msm/dsi: Use MSM and DRM DSC helper methods
Use MSM and DRM DSC helper methods to configure DSC for DSI. Reviewed-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/dsi/dsi_host.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 74d38f90398a..5526a51b3d97 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -28,6 +28,7 @@ #include "dsi.xml.h" #include "sfpb.xml.h" #include "dsi_cfg.h" +#include "msm_dsc_helper.h" #include "msm_kms.h" #include "msm_gem.h" #include "phy/dsi_phy.h" @@ -848,7 +849,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod /* first calculate dsc parameters and then program * compress mode registers */ - slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width); + slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay); /* * If slice_count is greater than slice_per_intf @@ -1759,7 +1760,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc return ret; } - dsc->initial_scale_value = 32; + dsc->initial_scale_value = drm_dsc_initial_scale_value(dsc); dsc->line_buf_depth = dsc->bits_per_component + 1; return drm_dsc_compute_rc_parameters(dsc); -- 2.40.1
[Freedreno] [PATCH v14 5/9] drm/msm: Add MSM-specific DSC helper methods
Introduce MSM-specific DSC helper methods, as some calculations are common between DP and DSC. Reviewed-by: Marijn Suijten Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/msm_dsc_helper.h | 38 1 file changed, 38 insertions(+) diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.h b/drivers/gpu/drm/msm/msm_dsc_helper.h new file mode 100644 index ..b9049fe1e279 --- /dev/null +++ b/drivers/gpu/drm/msm/msm_dsc_helper.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved + * + * Helper methods for MSM-specific DSC calculations that are common between timing engine, + * DSI, and DP. + */ + +#ifndef MSM_DSC_HELPER_H_ +#define MSM_DSC_HELPER_H_ + +#include +#include + +/** + * msm_dsc_get_slices_per_intf() - calculate number of slices per interface + * @dsc: Pointer to drm dsc config struct + * @intf_width: interface width in pixels + * Returns: Integer representing the number of slices for the given interface + */ +static inline u32 msm_dsc_get_slices_per_intf(const struct drm_dsc_config *dsc, u32 intf_width) +{ + return DIV_ROUND_UP(intf_width, dsc->slice_width); +} + +/** + * msm_dsc_get_bytes_per_line() - calculate bytes per line + * @dsc: Pointer to drm dsc config struct + * Returns: Integer value representing bytes per line. DSI and DP need + * to perform further calculations to turn this into pclk_per_intf, + * such as dividing by different values depending on if widebus is enabled. + */ +static inline u32 msm_dsc_get_bytes_per_line(const struct drm_dsc_config *dsc) +{ + return dsc->slice_count * dsc->slice_chunk_size; +} + +#endif /* MSM_DSC_HELPER_H_ */ -- 2.40.1
[Freedreno] [PATCH v14 9/9] drm/msm/dsi: update hdisplay calculation for dsi_timing_setup
Currently, hdisplay is being divided by 3 for DSC. However, this calculation only works for cases where BPP = 8. Update hdisplay calculation to be bytes_per_line / 3, so that it accounts for cases where BPP != 8. Reviewed-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 5526a51b3d97..9223d7ec5a73 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) * pulse width same */ h_total -= hdisplay; - hdisplay /= 3; + hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3; h_total += hdisplay; ha_end = ha_start + hdisplay; } -- 2.40.1
[Freedreno] [PATCH v14 7/9] drm/msm/dpu: Fix slice_last_group_size calculation
Correct the math for slice_last_group_size so that it matches the calculations downstream. Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC") Reviewed-by: Dmitry Baryshkov Reviewed-by: Marijn Suijten Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c index 3cad6a80af97..ea7d828b8812 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c @@ -56,9 +56,10 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc, if (is_cmd_mode) initial_lines += 1; - slice_last_group_size = 3 - (dsc->slice_width % 3); + slice_last_group_size = (dsc->slice_width + 2) % 3; + data = (initial_lines << 20); - data |= ((slice_last_group_size - 1) << 18); + data |= (slice_last_group_size << 18); /* bpp is 6.4 format, 4 LSBs bits are for fractional part */ data |= (dsc->bits_per_pixel << 8); data |= (dsc->block_pred_enable << 7); -- 2.40.1
[Freedreno] [PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper
Add helper to get the integer value of drm_dsc_config.bits_per_pixel Reviewed-by: Marijn Suijten Reviewed-by: Dmitry Baryshkov Signed-off-by: Jessica Zhang --- drivers/gpu/drm/display/drm_dsc_helper.c | 13 + include/drm/display/drm_dsc_helper.h | 1 + 2 files changed, 14 insertions(+) diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c index b31fe9849784..4424380c6cb6 100644 --- a/drivers/gpu/drm/display/drm_dsc_helper.c +++ b/drivers/gpu/drm/display/drm_dsc_helper.c @@ -1436,6 +1436,19 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg) } EXPORT_SYMBOL(drm_dsc_compute_rc_parameters); +/** + * drm_dsc_get_bpp_int() - Get integer bits per pixel value for the given DRM DSC config + * @vdsc_cfg: Pointer to DRM DSC config struct + * + * Return: Integer BPP value + */ +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg) +{ + WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf); + return vdsc_cfg->bits_per_pixel >> 4; +} +EXPORT_SYMBOL(drm_dsc_get_bpp_int); + /** * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config * @dsc: Pointer to DRM DSC config struct diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h index f4e18e5d077a..913aa2071232 100644 --- a/include/drm/display/drm_dsc_helper.h +++ b/include/drm/display/drm_dsc_helper.h @@ -27,6 +27,7 @@ int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg); u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc); u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc); +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg); #endif /* _DRM_DSC_HELPER_H_ */ -- 2.40.1
[Freedreno] [PATCH v14 0/9] Introduce MSM-specific DSC helpers
There are some overlap in calculations for MSM-specific DSC variables between DP and DSI. In addition, the calculations for initial_scale_value and det_thresh_flatness that are defined within the DSC 1.2 specifications, but aren't yet included in drm_dsc_helper.c. This series moves these calculations to a shared msm_dsc_helper.c file and defines drm_dsc_helper methods for initial_scale_value and det_thresh_flatness. Note: For now, the MSM specific helper methods are only called for the DSI path, but will called for DP once DSC 1.2 support for DP has been added. Depends on: "drm/i915: move DSC RC tables to drm_dsc_helper.c" [1] [1] https://patchwork.freedesktop.org/series/114472/ --- Changes in v14: - Added kernel docs and made DRM DSC helper functions (Jani) - Link to v13: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v13-0-d7581e7be...@quicinc.com Changes in v13: - Reworded comment doc for msm_dsc_get_slices_per_intf() - Changed intf_width to u32 - msm_dsc_calculate_slices_per_intf -> msm_dsc_get_slices_per_intf - Link to v12: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v12-0-9cdb7401f...@quicinc.com Changes in v12: - Reworded summary comment for msm_dsc_helper.h - msm_dsc_get_slices_per_intf -> msm_dsc_calculate_slices_per_intf - Corrected total_bytes_per_intf math in dsc_update_dsc_timing - Rebased on top of latest version of "drm/i915: move DSC RC tables to drm_dsc_helper.c" - Link to v11: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v11-0-30270e1ee...@quicinc.com Changes in v11: - Fixed mismatched return types - Moved MSM DSC helpers summary comment to under copyright - Moved msm_dsc_get_bpp_int() to drm_dsc_helper.h - Reworded MSM DSC helper comment docs for clarity - Added const keyword where applicable - Renamed msm_dsc_get_slice_per_intf to msm_dsc_get_slices_per_intf - Removed msm_dsc_get_slice_per_intf() - Reworded commit message for "drm/msm/dsi: update hdisplay calculation for dsi_timing_setup" - Link to v10: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v10-0-4cb21168c...@quicinc.com Changes in v10: - Removed msm_dsc_get_bytes_per_slice helper - Inlined msm_dsc_get_bytes_per_intf - Refactored drm_dsc_set_initial_scale_value() to be a pure function - Renamed DRM DSC initial_scale and flatness_det_thresh helpers - Removed msm_dsc_helpers.o from Makefile - Link to v9: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v9-0-87daeaec2...@quicinc.com Changes in v9: - Fixed incorrect math for msm_dsc_get_bytes_per_line() - Link to v8: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v8-0-2c9b2bb12...@quicinc.com Changes in v8: - *_bytes_per_soft_slice --> *_bytes_per_slice - Fixed comment doc formatting for MSM DSC helpers - Use slice_chunk_size in msm_dsc_get_bytes_per_line calculation - Reworded "drm/msm/dpu: Use DRM DSC helper for det_thresh_flatness" commit title for clarity - Picked up "Reviewed-by" tags - Added duplicate Signed-off-by tag to "drm/display/dsc: Add flatness and initial scale value calculations" to reflect patch history - Link to v7: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v7-0-df48a2c54...@quicinc.com Changes in v7: - Renamed msm_dsc_get_pclk_per_intf to msm_dsc_get_bytes_per_line - Removed duplicate msm_dsc_get_dce_bytes_per_line - Reworded commit message for "drm/msm/dpu: Use DRM DSC helper for det_thresh_flatness" - Dropped slice_per_pkt change (it will be included in the later "Add DSC v1.2 Support for DSI" series) - Picked up "drm/display/dsc: Add flatness and initial scale value calculations" and "drm/display/dsc: add helper to set semi-const parameters", which were dropped from "drm/i915: move DSC RC tables to drm_dsc_helper.c" series - Picked up "Reviewed-by" tags - Removed changelog in individual patches - Link to v6: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v6-0-cb7f59f0f...@quicinc.com Changes in v6: - Documented return values for MSM DSC helpers - Fixed dependency issue in msm_dsc_helper.c - Link to v5: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v5-0-0108401d7...@quicinc.com Changes in v5: - Added extra line at end of msm_dsc_helper.h - Simplified msm_dsc_get_bytes_per_soft_slice() math - Simplified and inlined msm_dsc_get_pclk_per_intf() math - "Fix calculations pkt_per_line" --> "... Fix calculation for pkt_per_line" - Split dsc->slice_width check into a separate patch - Picked up Dmitry's msm/dsi patch ("drm/msm/dsi: use new helpers for DSC setup") - Removed unused headers in MSM DSC helper files - Picked up Reviewed-by tags - Link to v4: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v4-0-1b79c78b3...@quicinc.com Changes in v4: - Changed msm_dsc_get_uncompressed_pclk_per_intf to msm_dsc_get_pclk_per_intf - Moved pclk_per_intf calculation for dsi_timing_setup to `if (msm_host->dsc)` block - Link to v3: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v3-0-6bec0d277...@quicinc.com Changes in v3: - Dropped src_bpp parameter
Re: [Freedreno] [PATCH] drm/msm/dp: add module parameter for PSR
Hi, On Wed, May 24, 2023 at 1:06 AM Dmitry Baryshkov wrote: > > On 24/05/2023 09:59, Johan Hovold wrote: > > On Tue, May 23, 2023 at 12:23:04PM -0700, Abhinav Kumar wrote: > >> On 5/23/2023 8:24 AM, Johan Hovold wrote: > >>> On Fri, May 12, 2023 at 09:13:04PM +0300, Dmitry Baryshkov wrote: > On 28/04/2023 02:28, Abhinav Kumar wrote: > > On sc7280 where eDP is the primary display, PSR is causing > > IGT breakage even for basic test cases like kms_atomic and > > kms_atomic_transition. Most often the issue starts with below > > stack so providing that as reference > > > > Call trace: > > > > ---[ end trace ]--- > > [drm-dp] dp_ctrl_push_idle: PUSH_IDLE pattern timedout > > > > Other basic use-cases still seem to work fine hence add a > > a module parameter to allow toggling psr enable/disable till > > PSR related issues are hashed out with IGT. > > For the reference: Bjorn reported that he has issues with VT on a > PSR-enabled laptops. This patch fixes the issue for him > >>> > >>> Module parameters are almost never warranted, and it is definitely not > >>> the right way to handle a broken implementation. > >>> > >>> I've just sent a revert that unconditionally disables PSR support until > >>> the implementation has been fixed: > >>> > >>> > >>> https://lore.kernel.org/lkml/20230523151646.28366-1-johan+lin...@kernel.org/ > >> > >> I dont completely agree with this. Even the virtual terminal case was > >> reported to be fixed by one user but not the other. So it was probably > >> something missed out either in validation or reproduction steps of the > >> user who reported it to be fixed OR the user who reported it not fixed. > >> That needs to be investigated now. > > > > Yes, there may still be some time left to fix it, but it's pretty damn > > annoying to find that an issue reported two months ago still is not > > fixed at 6.4-rc3. (I even waited to make the switch to 6.4 so that I > > would not have to spend time on this.) > > > > I didn't see any mail from Bjorn saying that the series that claimed to > > fix the VT issue actually did fix the VT issue. There's only the comment > > above from Dmitry suggesting that disabling this feature is the only way > > to get a working terminal back. > > Originally this issue was reported by Doug, and at [1] he reported that > an issue is fixed for him. So, for me it looks like we have hardware > where VT works and hardware where it doesn't. As I understand it, the problem was originally reported by Bjorn over IRC. When he reported the problem on VT2, Stephen Boyd confirmed that he could see the same problem on our hardware when using VT2. I confirmed I could reproduce and also tested the fix. I don't remember if Bjorn ever tested the fix. I think many of us assumed that it was the same issue so a fix for one person would also fix the other. > Doug, can you please confirm whether you can reproduce the PSR+VT issue > on 6.4-rc (without extra patches) or if the issue is fixed for you? > > [1] > https://lore.kernel.org/dri-devel/CAD=FV=vshmqptsqfwjviezeerms-veotmfozejasuc9zsmj...@mail.gmail.com/ Ugh. Unfortunately, it's not easy for me to test this particular feature directly on upstream Linux. :( I typically run with a ChromeOS root filesystem, which works pretty well with mainline. One place where it doesn't work with mainline is VT2. The VT2 feature for Chromebooks is implemented with "frecon" and takes advantage of a downstream patch that we've carried in the ChromeOS kernel trees for years allowing handoff of who the DRM "master" is. For testing the fix previously, I confirmed that I could reproduce the problem on our downstream kernel (which had the PSR patches picked) and that the fixes worked for me in that context. Ah, but it shouldn't be too hard to pick that one patch. Let's see if that works. I'll pick ("CHROMIUM: drm: Add drm_master_relax debugfs file (non-root set/drop master ioctls)"). Indeed, it works! OK, so with the top of Linus's tree (v6.4-rc3-17-g9d646009f65d) plus the CHROMIUM patch to enable my VT2, I can confirm that I don't see the issue. I can switch to VT2 and typing works fine. I will say that on herobrine the backlight is all messed up, but I assume that's an unrelated issue. -Doug > > Regressions happen and sometimes there are corner cases that are harder > > to find, but this is a breakage of a fundamental feature that was > > reported before the code was even merged into mainline. > > > >> We should have ideally gone with the modparam with the feature patches > >> itself knowing that it gets enabled for all sinks if PSR is supported. > > > > Modparams are things of the past should not be used to enable broken > > features so that some vendor can tick of their internal lists of > > features that have been "mainlined". > > We have had a history of using modparam with i915 and IIRC amdgpu / > radeon drivers to allow users to easily check
Re: [Freedreno] [PATCH v13 00/10] add DSC 1.2 dpu supports
Hi Dmitry/Marijn, Are you satisfy with this patch series or any more comments will be added? Thanks, kuogee On 5/22/2023 5:00 PM, Kuogee Hsieh wrote: This series adds the DPU side changes to support DSC 1.2 encoder. This was validated with both DSI DSC 1.2 panel and DP DSC 1.2 monitor. The DSI and DP parts will be pushed later on top of this change. This seriel is rebase on [1], [2] and catalog fixes from rev-4 of [3]. [1]: https://patchwork.freedesktop.org/series/116851/ [2]: https://patchwork.freedesktop.org/series/116615/ [3]: https://patchwork.freedesktop.org/series/112332/ Abhinav Kumar (2): drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets Kuogee Hsieh (8): drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0 drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG drm/msm/dpu: add support for DSC encoder v1.2 engine drm/msm/dpu: always clear every individual pending flush mask drm/msm/dpu: separate DSC flush update out of interface drm/msm/dpu: tear down DSC data path when DSC disabled drivers/gpu/drm/msm/Makefile | 1 + .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h| 7 + .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 11 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 + .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 + .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 51 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 24 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 35 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 33 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 11 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 14 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 15 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 387 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c| 9 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +- 19 files changed, 644 insertions(+), 29 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
Re: [Freedreno] (subset) [PATCH v4 0/9] drm: fdinfo memory stats
On 24/05/2023 18:10, Neil Armstrong wrote: Hi, On Mon, 15 May 2023 07:30:07 -0700, Rob Clark wrote: Similar motivation to other similar recent attempt[1]. But with an attempt to have some shared code for this. As well as documentation. It is probably a bit UMA-centric, I guess devices with VRAM might want some placement stats as well. But this seems like a reasonable start. Basic gputop support: https://patchwork.freedesktop.org/series/116236/ And already nvtop support: https://github.com/Syllo/nvtop/pull/204 [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [1/9] drm/docs: Fix usage stats typos https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0020582a8afe9a8570f80ec503c59bf049a616de [2/9] drm: Add common fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3f09a0cd4ea3b9d34495450d686227d48e7ec648 [3/9] drm/msm: Switch to fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=51d86ee5e07ccef85af04ee9850b0baa107999b6 [4/9] drm/amdgpu: Switch to fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=376c25f8ca47084c4f0aff0f14684780756ccef4 [5/9] drm: Add fdinfo memory stats https://cgit.freedesktop.org/drm/drm-misc/commit/?id=686b21b5f6ca2f8a716f9a4ade07246dbfb2713e [6/9] drm/msm: Add memory stats to fdinfo https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3e9757f5ddb98238226ad68a1609aa313de35adb [7/9] drm/doc: Relax fdinfo string constraints https://cgit.freedesktop.org/drm/drm-misc/commit/?id=90d63a150b85fd1debb9c01237fb78faee02746a Hmm no idea what happened, but I really applied v5 ! Neil
Re: [Freedreno] [PATCH v5 0/7] drm: fdinfo memory stats
Hi, On Wed, 24 May 2023 08:59:30 -0700, Rob Clark wrote: > From: Rob Clark > > Similar motivation to other similar recent attempt[1]. But with an > attempt to have some shared code for this. As well as documentation. > > It is probably a bit UMA-centric, I guess devices with VRAM might want > some placement stats as well. But this seems like a reasonable start. > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [1/7] drm/docs: Fix usage stats typos https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0020582a8afe9a8570f80ec503c59bf049a616de [2/7] drm: Add common fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3f09a0cd4ea3b9d34495450d686227d48e7ec648 [3/7] drm/msm: Switch to fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=51d86ee5e07ccef85af04ee9850b0baa107999b6 [4/7] drm/amdgpu: Switch to fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=376c25f8ca47084c4f0aff0f14684780756ccef4 [5/7] drm: Add fdinfo memory stats https://cgit.freedesktop.org/drm/drm-misc/commit/?id=686b21b5f6ca2f8a716f9a4ade07246dbfb2713e [6/7] drm/msm: Add memory stats to fdinfo https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3e9757f5ddb98238226ad68a1609aa313de35adb [7/7] drm/doc: Relax fdinfo string constraints https://cgit.freedesktop.org/drm/drm-misc/commit/?id=90d63a150b85fd1debb9c01237fb78faee02746a -- Neil
Re: [Freedreno] (subset) [PATCH v4 0/9] drm: fdinfo memory stats
Hi, On Mon, 15 May 2023 07:30:07 -0700, Rob Clark wrote: > Similar motivation to other similar recent attempt[1]. But with an > attempt to have some shared code for this. As well as documentation. > > It is probably a bit UMA-centric, I guess devices with VRAM might want > some placement stats as well. But this seems like a reasonable start. > > Basic gputop support: https://patchwork.freedesktop.org/series/116236/ > And already nvtop support: https://github.com/Syllo/nvtop/pull/204 > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [1/9] drm/docs: Fix usage stats typos https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0020582a8afe9a8570f80ec503c59bf049a616de [2/9] drm: Add common fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3f09a0cd4ea3b9d34495450d686227d48e7ec648 [3/9] drm/msm: Switch to fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=51d86ee5e07ccef85af04ee9850b0baa107999b6 [4/9] drm/amdgpu: Switch to fdinfo helper https://cgit.freedesktop.org/drm/drm-misc/commit/?id=376c25f8ca47084c4f0aff0f14684780756ccef4 [5/9] drm: Add fdinfo memory stats https://cgit.freedesktop.org/drm/drm-misc/commit/?id=686b21b5f6ca2f8a716f9a4ade07246dbfb2713e [6/9] drm/msm: Add memory stats to fdinfo https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3e9757f5ddb98238226ad68a1609aa313de35adb [7/9] drm/doc: Relax fdinfo string constraints https://cgit.freedesktop.org/drm/drm-misc/commit/?id=90d63a150b85fd1debb9c01237fb78faee02746a -- Neil
[Freedreno] [PATCH v5 7/7] drm/doc: Relax fdinfo string constraints
From: Rob Clark The restriction about no whitespace, etc, really only applies to the usage of strings in keys. Values can contain anything (other than newline). Signed-off-by: Rob Clark Acked-by: Tvrtko Ursulin Acked-by: Dave Airlie --- Documentation/gpu/drm-usage-stats.rst | 27 ++- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index d012eb56885e..fe35a291ff3e 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -17,41 +17,42 @@ wherever possible effort should still be made to standardise as much as possible. File format specification = - File shall contain one key value pair per one line of text. - Colon character (`:`) must be used to delimit keys and values. - All keys shall be prefixed with `drm-`. - Whitespace between the delimiter and first non-whitespace character shall be ignored when parsing. -- Neither keys or values are allowed to contain whitespace characters. +- Keys are not allowed to contain whitespace characters. - Numerical key value pairs can end with optional unit string. - Data type of the value is fixed as defined in the specification. Key types - 1. Mandatory, fully standardised. 2. Optional, fully standardised. 3. Driver specific. Data types -- - - Unsigned integer without defining the maximum value. -- - String excluding any above defined reserved characters or whitespace. +- - String excluding any above defined reserved characters or whitespace. +- - String. Mandatory fully standardised keys - -- drm-driver: +- drm-driver: String shall contain the name this driver registered as via the respective `struct drm_driver` data structure. Optional fully standardised keys Identification ^^ @@ -68,62 +69,62 @@ to the in kernel representation of `struct drm_file` instances. Uniqueness of the value shall be either globally unique, or unique within the scope of each device, in which case `drm-pdev` shall be present as well. Userspace should make sure to not double account any usage statistics by using the above described criteria in order to associate data to individual clients. Utilization ^^^ -- drm-engine-: ns +- drm-engine-: ns GPUs usually contain multiple execution engines. Each shall be given a stable -and unique name (str), with possible values documented in the driver specific +and unique name (keystr), with possible values documented in the driver specific documentation. Value shall be in specified time units which the respective GPU engine spent busy executing workloads belonging to this client. Values are not required to be constantly monotonic if it makes the driver implementation easier, but are required to catch up with the previously reported larger value within a reasonable period. Upon observing a value lower than what was previously read, userspace is expected to stay with that larger previous value until a monotonic update is seen. -- drm-engine-capacity-: +- drm-engine-capacity-: Engine identifier string must be the same as the one specified in the -drm-engine- tag and shall contain a greater than zero number in case the +drm-engine- tag and shall contain a greater than zero number in case the exported engine corresponds to a group of identical hardware engines. In the absence of this tag parser shall assume capacity of one. Zero capacity is not allowed. -- drm-cycles-: +- drm-cycles-: Engine identifier string must be the same as the one specified in the -drm-engine- tag and shall contain the number of busy cycles for the given +drm-engine- tag and shall contain the number of busy cycles for the given engine. Values are not required to be constantly monotonic if it makes the driver implementation easier, but are required to catch up with the previously reported larger value within a reasonable period. Upon observing a value lower than what was previously read, userspace is expected to stay with that larger previous value until a monotonic update is seen. -- drm-maxfreq-: [Hz|MHz|KHz] +- drm-maxfreq-: [Hz|MHz|KHz] Engine identifier string must be the same as the one specified in the -drm-engine- tag and shall contain the maximum frequency for the given -engine. Taken together with drm-cycles-, this can be used to calculate -percentage utilization of the engine, whereas drm-engine- only reflects +drm-engine- tag and shall contain the maximum frequency for the given +engine. Taken together with drm-cycles-, this can be used to calculate +percentage utilization of the engine, whereas drm-engine- only reflects time active without considering what frequency the engine is operating as a percentage of it's maximum frequency. Memory ^^ -
[Freedreno] [PATCH v5 6/7] drm/msm: Add memory stats to fdinfo
From: Rob Clark Use the new helper to export stats about memory usage. v2: Drop unintended hunk v3: Rebase Signed-off-by: Rob Clark Reviewed-by: Emil Velikov Acked-by: Dave Airlie --- drivers/gpu/drm/msm/msm_drv.c | 2 ++ drivers/gpu/drm/msm/msm_gem.c | 15 +++ 2 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 02fd6093f9b0..58264ff2c4b1 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1059,20 +1059,22 @@ static const struct drm_ioctl_desc msm_ioctls[] = { static void msm_show_fdinfo(struct drm_printer *p, struct drm_file *file) { struct drm_device *dev = file->minor->dev; struct msm_drm_private *priv = dev->dev_private; if (!priv->gpu) return; msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p); + + drm_show_memory_stats(p, file); } static const struct file_operations fops = { .owner = THIS_MODULE, DRM_GEM_FOPS, .show_fdinfo = drm_show_fdinfo, }; static const struct drm_driver msm_driver = { .driver_features= DRIVER_GEM | diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index db6c4e281d75..c32264234ea1 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -1089,34 +1089,49 @@ int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file, msm_gem_object_set_name(obj, "%s", name); ret = drm_gem_handle_create(file, obj, handle); /* drop reference from allocate - handle holds it now */ drm_gem_object_put(obj); return ret; } +static enum drm_gem_object_status msm_gem_status(struct drm_gem_object *obj) +{ + struct msm_gem_object *msm_obj = to_msm_bo(obj); + enum drm_gem_object_status status = 0; + + if (msm_obj->pages) + status |= DRM_GEM_OBJECT_RESIDENT; + + if (msm_obj->madv == MSM_MADV_DONTNEED) + status |= DRM_GEM_OBJECT_PURGEABLE; + + return status; +} + static const struct vm_operations_struct vm_ops = { .fault = msm_gem_fault, .open = drm_gem_vm_open, .close = drm_gem_vm_close, }; static const struct drm_gem_object_funcs msm_gem_object_funcs = { .free = msm_gem_free_object, .pin = msm_gem_prime_pin, .unpin = msm_gem_prime_unpin, .get_sg_table = msm_gem_prime_get_sg_table, .vmap = msm_gem_prime_vmap, .vunmap = msm_gem_prime_vunmap, .mmap = msm_gem_object_mmap, + .status = msm_gem_status, .vm_ops = _ops, }; static int msm_gem_new_impl(struct drm_device *dev, uint32_t size, uint32_t flags, struct drm_gem_object **obj) { struct msm_drm_private *priv = dev->dev_private; struct msm_gem_object *msm_obj; -- 2.40.1
[Freedreno] [PATCH v5 4/7] drm/amdgpu: Switch to fdinfo helper
From: Rob Clark v2: Rebase on drm-misc-next Signed-off-by: Rob Clark Reviewed-by: Christian König Acked-by: Dave Airlie --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 32 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h | 2 +- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index b1ca1ab6d6ad..1b46e7ac7cb0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2740,21 +2740,21 @@ static const struct file_operations amdgpu_driver_kms_fops = { .flush = amdgpu_flush, .release = drm_release, .unlocked_ioctl = amdgpu_drm_ioctl, .mmap = drm_gem_mmap, .poll = drm_poll, .read = drm_read, #ifdef CONFIG_COMPAT .compat_ioctl = amdgpu_kms_compat_ioctl, #endif #ifdef CONFIG_PROC_FS - .show_fdinfo = amdgpu_show_fdinfo + .show_fdinfo = drm_show_fdinfo, #endif }; int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv) { struct drm_file *file; if (!filp) return -EINVAL; @@ -2795,20 +2795,21 @@ static const struct drm_driver amdgpu_kms_driver = { DRIVER_SYNCOBJ_TIMELINE, .open = amdgpu_driver_open_kms, .postclose = amdgpu_driver_postclose_kms, .lastclose = amdgpu_driver_lastclose_kms, .ioctls = amdgpu_ioctls_kms, .num_ioctls = ARRAY_SIZE(amdgpu_ioctls_kms), .dumb_create = amdgpu_mode_dumb_create, .dumb_map_offset = amdgpu_mode_dumb_mmap, .fops = _driver_kms_fops, .release = _driver_release_kms, + .show_fdinfo = amdgpu_show_fdinfo, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, .gem_prime_import = amdgpu_gem_prime_import, .gem_prime_mmap = drm_gem_prime_mmap, .name = DRIVER_NAME, .desc = DRIVER_DESC, .date = DRIVER_DATE, .major = KMS_DRIVER_MAJOR, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c index c57252f004e8..13d7413d4ca3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c @@ -46,23 +46,22 @@ static const char *amdgpu_ip_name[AMDGPU_HW_IP_NUM] = { [AMDGPU_HW_IP_COMPUTE] = "compute", [AMDGPU_HW_IP_DMA] = "dma", [AMDGPU_HW_IP_UVD] = "dec", [AMDGPU_HW_IP_VCE] = "enc", [AMDGPU_HW_IP_UVD_ENC] = "enc_1", [AMDGPU_HW_IP_VCN_DEC] = "dec", [AMDGPU_HW_IP_VCN_ENC] = "enc", [AMDGPU_HW_IP_VCN_JPEG] = "jpeg", }; -void amdgpu_show_fdinfo(struct seq_file *m, struct file *f) +void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file) { - struct drm_file *file = f->private_data; struct amdgpu_device *adev = drm_to_adev(file->minor->dev); struct amdgpu_fpriv *fpriv = file->driver_priv; struct amdgpu_vm *vm = >vm; struct amdgpu_mem_stats stats; ktime_t usage[AMDGPU_HW_IP_NUM]; uint32_t bus, dev, fn, domain; unsigned int hw_ip; int ret; @@ -80,38 +79,37 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f) amdgpu_bo_unreserve(vm->root.bo); amdgpu_ctx_mgr_usage(>ctx_mgr, usage); /* * ** * For text output format description please see drm-usage-stats.rst! * ** */ - seq_printf(m, "pasid:\t%u\n", fpriv->vm.pasid); - seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name); - seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\n", domain, bus, dev, fn); - seq_printf(m, "drm-client-id:\t%Lu\n", vm->immediate.fence_context); - seq_printf(m, "drm-memory-vram:\t%llu KiB\n", stats.vram/1024UL); - seq_printf(m, "drm-memory-gtt: \t%llu KiB\n", stats.gtt/1024UL); - seq_printf(m, "drm-memory-cpu: \t%llu KiB\n", stats.cpu/1024UL); - seq_printf(m, "amd-memory-visible-vram:\t%llu KiB\n", + drm_printf(p, "pasid:\t%u\n", fpriv->vm.pasid); + drm_printf(p, "drm-driver:\t%s\n", file->minor->dev->driver->name); + drm_printf(p, "drm-pdev:\t%04x:%02x:%02x.%d\n", domain, bus, dev, fn); + drm_printf(p, "drm-client-id:\t%Lu\n", vm->immediate.fence_context); + drm_printf(p, "drm-memory-vram:\t%llu KiB\n", stats.vram/1024UL); + drm_printf(p, "drm-memory-gtt: \t%llu KiB\n", stats.gtt/1024UL); + drm_printf(p, "drm-memory-cpu: \t%llu KiB\n", stats.cpu/1024UL); + drm_printf(p, "amd-memory-visible-vram:\t%llu KiB\n", stats.visible_vram/1024UL); - seq_printf(m,
[Freedreno] [PATCH v5 5/7] drm: Add fdinfo memory stats
From: Rob Clark Add support to dump GEM stats to fdinfo. v2: Fix typos, change size units to match docs, use div_u64 v3: Do it in core v4: more kerneldoc v5: doc fixes v6: Actually use u64, bit more comment docs Signed-off-by: Rob Clark Reviewed-by: Emil Velikov Reviewed-by: Daniel Vetter Acked-by: Tvrtko Ursulin Acked-by: Dave Airlie --- Documentation/gpu/drm-usage-stats.rst | 54 +++ drivers/gpu/drm/drm_file.c| 99 ++- include/drm/drm_file.h| 28 include/drm/drm_gem.h | 32 + 4 files changed, 200 insertions(+), 13 deletions(-) diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index 552195fb1ea3..d012eb56885e 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -45,37 +45,43 @@ Mandatory fully standardised keys - - drm-driver: String shall contain the name this driver registered as via the respective `struct drm_driver` data structure. Optional fully standardised keys +Identification +^^ + - drm-pdev: For PCI devices this should contain the PCI slot address of the device in question. - drm-client-id: Unique value relating to the open DRM file descriptor used to distinguish duplicated and shared file descriptors. Conceptually the value should map 1:1 to the in kernel representation of `struct drm_file` instances. Uniqueness of the value shall be either globally unique, or unique within the scope of each device, in which case `drm-pdev` shall be present as well. Userspace should make sure to not double account any usage statistics by using the above described criteria in order to associate data to individual clients. +Utilization +^^^ + - drm-engine-: ns GPUs usually contain multiple execution engines. Each shall be given a stable and unique name (str), with possible values documented in the driver specific documentation. Value shall be in specified time units which the respective GPU engine spent busy executing workloads belonging to this client. Values are not required to be constantly monotonic if it makes the driver @@ -86,32 +92,20 @@ value until a monotonic update is seen. - drm-engine-capacity-: Engine identifier string must be the same as the one specified in the drm-engine- tag and shall contain a greater than zero number in case the exported engine corresponds to a group of identical hardware engines. In the absence of this tag parser shall assume capacity of one. Zero capacity is not allowed. -- drm-memory-: [KiB|MiB] - -Each possible memory type which can be used to store buffer objects by the -GPU in question shall be given a stable and unique name to be returned as the -string here. - -Value shall reflect the amount of storage currently consumed by the buffer -object belong to this client, in the respective memory region. - -Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' -indicating kibi- or mebi-bytes. - - drm-cycles-: Engine identifier string must be the same as the one specified in the drm-engine- tag and shall contain the number of busy cycles for the given engine. Values are not required to be constantly monotonic if it makes the driver implementation easier, but are required to catch up with the previously reported larger value within a reasonable period. Upon observing a value lower than what was previously read, userspace is expected to stay with that larger previous @@ -119,20 +113,56 @@ value until a monotonic update is seen. - drm-maxfreq-: [Hz|MHz|KHz] Engine identifier string must be the same as the one specified in the drm-engine- tag and shall contain the maximum frequency for the given engine. Taken together with drm-cycles-, this can be used to calculate percentage utilization of the engine, whereas drm-engine- only reflects time active without considering what frequency the engine is operating as a percentage of it's maximum frequency. +Memory +^^ + +- drm-memory-: [KiB|MiB] + +Each possible memory type which can be used to store buffer objects by the +GPU in question shall be given a stable and unique name to be returned as the +string here. The name "memory" is reserved to refer to normal system memory. + +Value shall reflect the amount of storage currently consumed by the buffer +objects belong to this client, in the respective memory region. + +Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' +indicating kibi- or mebi-bytes. + +- drm-shared-: [KiB|MiB] + +The total size of buffers that are shared with another file (ie. have more +than a single handle). + +- drm-total-: [KiB|MiB] + +The total size of buffers that including shared and private memory. + +- drm-resident-: [KiB|MiB] + +The total size of buffers that are resident in the
[Freedreno] [PATCH v5 2/7] drm: Add common fdinfo helper
From: Rob Clark Handle a bit of the boiler-plate in a single case, and make it easier to add some core tracked stats. This also ensures consistent behavior across drivers for standardised fields. v2: Update drm-usage-stats.rst, 64b client-id, rename drm_show_fdinfo v3: Rebase on drm-misc-next Reviewed-by: Daniel Vetter Signed-off-by: Rob Clark Acked-by: Dave Airlie --- Documentation/gpu/drm-usage-stats.rst | 10 +++- drivers/gpu/drm/drm_file.c| 35 +++ include/drm/drm_drv.h | 7 ++ include/drm/drm_file.h| 4 +++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index 72d069e5dacb..552195fb1ea3 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -119,14 +119,22 @@ value until a monotonic update is seen. - drm-maxfreq-: [Hz|MHz|KHz] Engine identifier string must be the same as the one specified in the drm-engine- tag and shall contain the maximum frequency for the given engine. Taken together with drm-cycles-, this can be used to calculate percentage utilization of the engine, whereas drm-engine- only reflects time active without considering what frequency the engine is operating as a percentage of it's maximum frequency. +Implementation Details +== + +Drivers should use drm_show_fdinfo() in their `struct file_operations`, and +implement _driver.show_fdinfo if they wish to provide any stats which +are not provided by drm_show_fdinfo(). But even driver specific stats should +be documented above and where possible, aligned with other drivers. + Driver specific implementations -=== +--- :ref:`i915-usage-stats` diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index c1018c470047..37b4f76a5191 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -141,28 +141,31 @@ bool drm_dev_needs_global_mutex(struct drm_device *dev) * * This allocates a new DRM file context. It is not linked into any context and * can be used by the caller freely. Note that the context keeps a pointer to * @minor, so it must be freed before @minor is. * * RETURNS: * Pointer to newly allocated context, ERR_PTR on failure. */ struct drm_file *drm_file_alloc(struct drm_minor *minor) { + static atomic64_t ident = ATOMIC_INIT(0); struct drm_device *dev = minor->dev; struct drm_file *file; int ret; file = kzalloc(sizeof(*file), GFP_KERNEL); if (!file) return ERR_PTR(-ENOMEM); + /* Get a unique identifier for fdinfo: */ + file->client_id = atomic64_inc_return(); file->pid = get_pid(task_tgid(current)); file->minor = minor; /* for compatibility root is always authenticated */ file->authenticated = capable(CAP_SYS_ADMIN); INIT_LIST_HEAD(>lhead); INIT_LIST_HEAD(>fbs); mutex_init(>fbs_lock); INIT_LIST_HEAD(>blobs); @@ -861,20 +864,52 @@ EXPORT_SYMBOL(drm_send_event_locked); void drm_send_event(struct drm_device *dev, struct drm_pending_event *e) { unsigned long irqflags; spin_lock_irqsave(>event_lock, irqflags); drm_send_event_helper(dev, e, 0); spin_unlock_irqrestore(>event_lock, irqflags); } EXPORT_SYMBOL(drm_send_event); +/** + * drm_show_fdinfo - helper for drm file fops + * @seq_file: output stream + * @f: the device file instance + * + * Helper to implement fdinfo, for userspace to query usage stats, etc, of a + * process using the GPU. See also _driver.show_fdinfo. + * + * For text output format description please see Documentation/gpu/drm-usage-stats.rst + */ +void drm_show_fdinfo(struct seq_file *m, struct file *f) +{ + struct drm_file *file = f->private_data; + struct drm_device *dev = file->minor->dev; + struct drm_printer p = drm_seq_file_printer(m); + + drm_printf(, "drm-driver:\t%s\n", dev->driver->name); + drm_printf(, "drm-client-id:\t%llu\n", file->client_id); + + if (dev_is_pci(dev->dev)) { + struct pci_dev *pdev = to_pci_dev(dev->dev); + + drm_printf(, "drm-pdev:\t%04x:%02x:%02x.%d\n", + pci_domain_nr(pdev->bus), pdev->bus->number, + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); + } + + if (dev->driver->show_fdinfo) + dev->driver->show_fdinfo(, file); +} +EXPORT_SYMBOL(drm_show_fdinfo); + /** * mock_drm_getfile - Create a new struct file for the drm device * @minor: drm minor to wrap (e.g. #drm_device.primary) * @flags: file creation mode (O_RDWR etc) * * This create a new struct file that wraps a DRM file context around a * DRM minor. This mimicks userspace opening e.g. /dev/dri/card0, but without * invoking userspace. The
[Freedreno] [PATCH v5 3/7] drm/msm: Switch to fdinfo helper
From: Rob Clark Now that we have a common helper, use it. v2: Rebase on drm-misc-next Signed-off-by: Rob Clark Reviewed-by: Dmitry Baryshkov Acked-by: Dave Airlie --- drivers/gpu/drm/msm/msm_drv.c | 11 +-- drivers/gpu/drm/msm/msm_gpu.c | 2 -- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 060c7689a739..02fd6093f9b0 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1050,56 +1050,55 @@ static const struct drm_ioctl_desc msm_ioctls[] = { DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_PREP, msm_ioctl_gem_cpu_prep, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_FINI, msm_ioctl_gem_cpu_fini, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(MSM_GEM_SUBMIT, msm_ioctl_gem_submit, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(MSM_WAIT_FENCE, msm_ioctl_wait_fence, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(MSM_GEM_MADVISE, msm_ioctl_gem_madvise, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_NEW, msm_ioctl_submitqueue_new, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_CLOSE, msm_ioctl_submitqueue_close, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, DRM_RENDER_ALLOW), }; -static void msm_fop_show_fdinfo(struct seq_file *m, struct file *f) +static void msm_show_fdinfo(struct drm_printer *p, struct drm_file *file) { - struct drm_file *file = f->private_data; struct drm_device *dev = file->minor->dev; struct msm_drm_private *priv = dev->dev_private; - struct drm_printer p = drm_seq_file_printer(m); if (!priv->gpu) return; - msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, ); + msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p); } static const struct file_operations fops = { .owner = THIS_MODULE, DRM_GEM_FOPS, - .show_fdinfo = msm_fop_show_fdinfo, + .show_fdinfo = drm_show_fdinfo, }; static const struct drm_driver msm_driver = { .driver_features= DRIVER_GEM | DRIVER_RENDER | DRIVER_ATOMIC | DRIVER_MODESET | DRIVER_SYNCOBJ, .open = msm_open, - .postclose = msm_postclose, + .postclose = msm_postclose, .dumb_create= msm_gem_dumb_create, .dumb_map_offset= msm_gem_dumb_map_offset, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, .gem_prime_import_sg_table = msm_gem_prime_import_sg_table, .gem_prime_mmap = msm_gem_prime_mmap, #ifdef CONFIG_DEBUG_FS .debugfs_init = msm_debugfs_init, #endif + .show_fdinfo= msm_show_fdinfo, .ioctls = msm_ioctls, .num_ioctls = ARRAY_SIZE(msm_ioctls), .fops = , .name = "msm", .desc = "MSM Snapdragon DRM", .date = "20130625", .major = MSM_VERSION_MAJOR, .minor = MSM_VERSION_MINOR, .patchlevel = MSM_VERSION_PATCHLEVEL, }; diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 26ebda40be4f..c403912d13ab 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -144,22 +144,20 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu) return ret; gpu->suspend_count++; return 0; } void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx, struct drm_printer *p) { - drm_printf(p, "drm-driver:\t%s\n", gpu->dev->driver->name); - drm_printf(p, "drm-client-id:\t%u\n", ctx->seqno); drm_printf(p, "drm-engine-gpu:\t%llu ns\n", ctx->elapsed_ns); drm_printf(p, "drm-cycles-gpu:\t%llu\n", ctx->cycles); drm_printf(p, "drm-maxfreq-gpu:\t%u Hz\n", gpu->fast_rate); } int msm_gpu_hw_init(struct msm_gpu *gpu) { int ret; WARN_ON(!mutex_is_locked(>lock)); -- 2.40.1
[Freedreno] [PATCH v5 1/7] drm/docs: Fix usage stats typos
From: Rob Clark Fix a couple missing ':'s. Signed-off-by: Rob Clark Reviewed-by: Rodrigo Vivi Acked-by: Dave Airlie --- Documentation/gpu/drm-usage-stats.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index b46327356e80..72d069e5dacb 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -98,33 +98,33 @@ is not allowed. Each possible memory type which can be used to store buffer objects by the GPU in question shall be given a stable and unique name to be returned as the string here. Value shall reflect the amount of storage currently consumed by the buffer object belong to this client, in the respective memory region. Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' indicating kibi- or mebi-bytes. -- drm-cycles- +- drm-cycles-: Engine identifier string must be the same as the one specified in the drm-engine- tag and shall contain the number of busy cycles for the given engine. Values are not required to be constantly monotonic if it makes the driver implementation easier, but are required to catch up with the previously reported larger value within a reasonable period. Upon observing a value lower than what was previously read, userspace is expected to stay with that larger previous value until a monotonic update is seen. -- drm-maxfreq- [Hz|MHz|KHz] +- drm-maxfreq-: [Hz|MHz|KHz] Engine identifier string must be the same as the one specified in the drm-engine- tag and shall contain the maximum frequency for the given engine. Taken together with drm-cycles-, this can be used to calculate percentage utilization of the engine, whereas drm-engine- only reflects time active without considering what frequency the engine is operating as a percentage of it's maximum frequency. Driver specific implementations === -- 2.40.1
[Freedreno] [PATCH v5 0/7] drm: fdinfo memory stats
From: Rob Clark Similar motivation to other similar recent attempt[1]. But with an attempt to have some shared code for this. As well as documentation. It is probably a bit UMA-centric, I guess devices with VRAM might want some placement stats as well. But this seems like a reasonable start. Basic gputop support: https://patchwork.freedesktop.org/series/116236/ And already nvtop support: https://github.com/Syllo/nvtop/pull/204 I've combined the separate series to add comm/cmdline override onto the end of this, simply out of convenience (they would otherwise conflict in a bunch of places). v2: Extend things to allow for multiple regions other than just system "memory", make drm_show_memory_stats() a helper so that, drivers can use it or not based on their needs (but in either case, re- use drm_print_memory_stats() v3: Docs fixes v4: use u64 for drm_memory_stats, small docs update and collected Tvrtko's a-b v5: Rebase on drm-misc-next, drop comm/cmdline override patches [1] https://patchwork.freedesktop.org/series/112397/ Rob Clark (7): drm/docs: Fix usage stats typos drm: Add common fdinfo helper drm/msm: Switch to fdinfo helper drm/amdgpu: Switch to fdinfo helper drm: Add fdinfo memory stats drm/msm: Add memory stats to fdinfo drm/doc: Relax fdinfo string constraints Documentation/gpu/drm-usage-stats.rst | 91 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 32 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h | 2 +- drivers/gpu/drm/drm_file.c | 132 + drivers/gpu/drm/msm/msm_drv.c | 13 +- drivers/gpu/drm/msm/msm_gem.c | 15 +++ drivers/gpu/drm/msm/msm_gpu.c | 2 - include/drm/drm_drv.h | 7 ++ include/drm/drm_file.h | 32 + include/drm/drm_gem.h | 32 + 11 files changed, 308 insertions(+), 53 deletions(-) -- 2.40.1
Re: [Freedreno] [PATCH] drm/msm/dp: add module parameter for PSR
On Wed, May 24, 2023 at 11:06:03AM +0300, Dmitry Baryshkov wrote: > On 24/05/2023 09:59, Johan Hovold wrote: > > Regressions happen and sometimes there are corner cases that are harder > > to find, but this is a breakage of a fundamental feature that was > > reported before the code was even merged into mainline. > > > >> We should have ideally gone with the modparam with the feature patches > >> itself knowing that it gets enabled for all sinks if PSR is supported. > > > > Modparams are things of the past should not be used to enable broken > > features so that some vendor can tick of their internal lists of > > features that have been "mainlined". > > We have had a history of using modparam with i915 and IIRC amdgpu / > radeon drivers to allow users to easily check whether new feature works > for their hardware. My current understanding is that PSR+VT works for on > some laptops and doesn't on some other laptops, which makes it a valid case. But here it does not seem to be the hardware that's the issue, but rather that the implementation is incorrect or incomplete. Johan
Re: [Freedreno] [PATCH] Revert "drm/msm/dp: Remove INIT_SETUP delay"
>> [ 275.025497] [drm:dpu_encoder_phys_vid_wait_for_commit_done:488] >> [dpu error]vblank timeout >> [ 275.025514] [drm:dpu_kms_wait_for_commit_done:510] [dpu error]wait >> for commit done returned -110 >> [ 275.064141] [drm:dpu_encoder_frame_done_timeout:2382] [dpu >> error]enc33 frame done timeout This is a different crash but the root-cause of both the issues is the bridge hpd_enable/disable series. https://patchwork.freedesktop.org/patch/514414/ >> >> Yes, the new patch to fix this issue is here >> >> https://patchwork.freedesktop.org/patch/538601/?series=118148=3 >> >> Apologies if you were not CCed on this, if a next version is CCed, >> will ask kuogee to cc you. >> >> Meanwhile, will be great if you can verify if it works for you and >> provide Tested-by tags. > > Hi Leonard, > > I had cc you with v5 patches. > > Would you please verify it. Hi Kuogee, thank you. Verified the v6 patch fixes the regression when ported to 6.3.3. One non-fatal issue remains: Suspending and resuming the system while USB-C DP monitor is connected triggers an error, though the system recovers within a second without the need to unplug the cable. [drm:drm_mode_config_helper_resume] *ERROR* Failed to resume (-107) dmesg snippet related to the suspend below [ 194.066321] PM: suspend entry (deep) [ 194.178793] Filesystems sync: 0.108 seconds [ 194.184142] LoadPin: firmware pinning-ignored obj="/usr/lib/firmware/qcom/sc7180-trogdor/modem-nolte/qdsp6sw.mbn" pid=3380 cmdline="" [ 194.196934] LoadPin: firmware pinning-ignored obj="/usr/lib/firmware/qcom/sc7180-trogdor/modem-nolte/mba.mbn" pid=3387 cmdline="" [ 194.197320] LoadPin: firmware pinning-ignored obj="/usr/lib/firmware/regulatory.db-debian" pid=3390 cmdline="" [ 194.204128] LoadPin: firmware pinning-ignored obj="/usr/lib/firmware/qcom/venus-5.4/venus.mbn" pid=3380 cmdline="" [ 194.204808] LoadPin: firmware pinning-ignored obj="/usr/lib/firmware/qca/crbtfw32.tlv" pid=3380 cmdline="" [ 194.205058] LoadPin: firmware pinning-ignored obj="/usr/lib/firmware/qca/crnv32.bin" pid=3380 cmdline="" [ 194.253591] Freezing user space processes [ 194.263621] Freezing user space processes completed (elapsed 0.005 seconds) [ 194.270816] OOM killer disabled. [ 194.274165] Freezing remaining freezable tasks [ 194.281253] Freezing remaining freezable tasks completed (elapsed 0.002 seconds) [ 194.288866] printk: Suspending console(s) (use no_console_suspend to debug) [ 194.494479] Disabling non-boot CPUs ... [ 194.497569] psci: CPU1 killed (polled 1 ms) [ 194.501844] psci: CPU2 killed (polled 1 ms) [ 194.506311] psci: CPU3 killed (polled 1 ms) [ 194.510237] psci: CPU4 killed (polled 1 ms) [ 194.512854] psci: CPU5 killed (polled 1 ms) [ 194.516076] psci: CPU6 killed (polled 1 ms) [ 194.518397] psci: CPU7 killed (polled 0 ms) [ 194.520706] Enabling non-boot CPUs ... [ 194.521595] Detected VIPT I-cache on CPU1 [ 194.521664] cacheinfo: Unable to detect cache hierarchy for CPU 1 [ 194.521678] GICv3: CPU1: found redistributor 100 region 0:0x17a8 [ 194.521743] CPU1: Booted secondary processor 0x000100 [0x51df805e] [ 194.522829] CPU1 is up [ 194.523646] Detected VIPT I-cache on CPU2 [ 194.523701] cacheinfo: Unable to detect cache hierarchy for CPU 2 [ 194.523716] GICv3: CPU2: found redistributor 200 region 0:0x17aa [ 194.523775] CPU2: Booted secondary processor 0x000200 [0x51df805e] [ 194.524809] CPU2 is up [ 194.525537] Detected VIPT I-cache on CPU3 [ 194.525592] cacheinfo: Unable to detect cache hierarchy for CPU 3 [ 194.525611] GICv3: CPU3: found redistributor 300 region 0:0x17ac [ 194.525668] CPU3: Booted secondary processor 0x000300 [0x51df805e] [ 194.526674] CPU3 is up [ 194.527486] Detected VIPT I-cache on CPU4 [ 194.527535] cacheinfo: Unable to detect cache hierarchy for CPU 4 [ 194.527556] GICv3: CPU4: found redistributor 400 region 0:0x17ae [ 194.527612] CPU4: Booted secondary processor 0x000400 [0x51df805e] [ 194.528836] CPU4 is up [ 194.529553] Detected VIPT I-cache on CPU5 [ 194.529601] cacheinfo: Unable to detect cache hierarchy for CPU 5 [ 194.529623] GICv3: CPU5: found redistributor 500 region 0:0x17b0 [ 194.529675] CPU5: Booted secondary processor 0x000500 [0x51df805e] [ 194.530986] CPU5 is up [ 194.532280] Detected PIPT I-cache on CPU6 [ 194.532307] cacheinfo: Unable to detect cache hierarchy for CPU 6 [ 194.532322] GICv3: CPU6: found redistributor 600 region 0:0x17b2 [ 194.532358] CPU6: Booted secondary processor 0x000600 [0x51ff804f] [ 194.534434] CPU6 is up [ 194.535408] Detected PIPT I-cache on CPU7 [ 194.535445] cacheinfo: Unable to detect cache hierarchy for CPU 7 [ 194.535463] GICv3: CPU7: found redistributor 700 region 0:0x17b4 [ 194.535505] CPU7: Booted secondary processor 0x000700 [0x51ff804f] [ 194.536281] CPU7 is up [ 195.285023]
Re: [Freedreno] [PATCH v6] drm/msm/dp: enable HDP plugin/unplugged interrupts at hpd_enable/disable
Kuogee Hsieh writes: > The internal_hpd flag is set to true by dp_bridge_hpd_enable() and set to > false by dp_bridge_hpd_disable() to handle GPIO pinmuxed into DP controller > case. HDP related interrupts can not be enabled until internal_hpd is set > to true. At current implementation dp_display_config_hpd() will initialize > DP host controller first followed by enabling HDP related interrupts if > internal_hpd was true at that time. Enable HDP related interrupts depends on > internal_hpd status may leave system with DP driver host is in running state > but without HDP related interrupts being enabled. This will prevent external > display from being detected. Eliminated this dependency by moving HDP related > interrupts enable/disable be done at dp_bridge_hpd_enable/disable() directly > regardless of internal_hpd status. > > Changes in V3: > -- dp_catalog_ctrl_hpd_enable() and dp_catalog_ctrl_hpd_disable() > -- rewording ocmmit text > > Changes in V4: > -- replace dp_display_config_hpd() with dp_display_host_start() > -- move enable_irq() at dp_display_host_start(); > > Changes in V5: > -- replace dp_display_host_start() with dp_display_host_init() > > Changes in V6: > -- squash remove enable_irq() and disable_irq() > > Fixes: cd198caddea7 ("drm/msm/dp: Rely on hpd_enable/disable callbacks") > Signed-off-by: Kuogee Hsieh Cherry-picked on top of v6.3.3 and verified fixes the USB-C DP regression on sc7180 lazor. Thank you! Tested-by: Leonard Lausen # on sc7180 lazor
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
On Wed, 24 May 2023 at 12:48, Marijn Suijten wrote: > > On 2023-05-23 13:01:13, Abhinav Kumar wrote: > > > > > > On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote: > > > Drop SSPP-specifig debugfs register dumps in favour of using > > > debugfs/dri/0/kms or devcoredump. > > > > > > > I did see another series which removes src_blk from the catalog (I am > > yet to review that one) . Lets assume that one is fine and this change > > will be going on top of that one right? > > It replaces src_blk with directly accessing the blk (non-sub-block) > directly, because they were overlapping anyway. > > > The concern I have with this change is that although I do agree that we > > should be in favor of using debugfs/dri/0/kms ( i have used it a few > > times and it works pretty well ), devcoredump does not have the support > > to dump sub-blocks . Something which we should add with priority because > > even with DSC blocks with the separation of enc/ctl blocks we need that > > like I wrote in one of the responses. > > > > So the "len" of the blocks having sub-blocks will be ignored in favor of > > the len of the sub-blocks. > > The sub-blocks are not always contiguous with their parent block, are > they? It's probably better to print the sub-blocks separately with > clear headers anyway I hope this is what Abhinav meant. > rather than dumping the range parent_blk_base to > max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...). > > - Marijn > > > If we remove this without adding that support first, its a loss of debug > > functionality. > > > > Can we retain these blocks and remove dpu_debugfs_create_regset32 in a > > different way? > > -- With best wishes Dmitry
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
On 2023-05-23 13:01:13, Abhinav Kumar wrote: > > > On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote: > > Drop SSPP-specifig debugfs register dumps in favour of using > > debugfs/dri/0/kms or devcoredump. > > > > I did see another series which removes src_blk from the catalog (I am > yet to review that one) . Lets assume that one is fine and this change > will be going on top of that one right? It replaces src_blk with directly accessing the blk (non-sub-block) directly, because they were overlapping anyway. > The concern I have with this change is that although I do agree that we > should be in favor of using debugfs/dri/0/kms ( i have used it a few > times and it works pretty well ), devcoredump does not have the support > to dump sub-blocks . Something which we should add with priority because > even with DSC blocks with the separation of enc/ctl blocks we need that > like I wrote in one of the responses. > > So the "len" of the blocks having sub-blocks will be ignored in favor of > the len of the sub-blocks. The sub-blocks are not always contiguous with their parent block, are they? It's probably better to print the sub-blocks separately with clear headers anyway rather than dumping the range parent_blk_base to max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...). - Marijn > If we remove this without adding that support first, its a loss of debug > functionality. > > Can we retain these blocks and remove dpu_debugfs_create_regset32 in a > different way?
[Freedreno] [PATCH v4 11/13] drm/fb-helper: Export helpers for marking damage areas
Export drm_fb_helper_damage() and drm_fb_helper_damage_range(), which handle damage areas for fbdev emulation. This is a temporary export that allows to move the DRM I/O helpers for fbdev into drivers. Only fbdev-generic and i915 need them. Both will be updated to implement damage handling by themselves and the exported functions will be removed. v4: * update interfaces Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_fb_helper.c | 22 ++ include/drm/drm_fb_helper.h | 3 +++ 2 files changed, 25 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index f0e9549b6bd7..cb03099fd2e3 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -670,6 +670,28 @@ static void drm_fb_helper_memory_range_to_clip(struct fb_info *info, off_t off, drm_rect_init(clip, x1, y1, x2 - x1, y2 - y1); } +/* Don't use in new code. */ +void drm_fb_helper_damage_range(struct fb_info *info, off_t off, size_t len) +{ + struct drm_fb_helper *fb_helper = info->par; + struct drm_rect damage_area; + + drm_fb_helper_memory_range_to_clip(info, off, len, _area); + drm_fb_helper_damage(fb_helper, damage_area.x1, damage_area.y1, +drm_rect_width(_area), +drm_rect_height(_area)); +} +EXPORT_SYMBOL(drm_fb_helper_damage_range); + +/* Don't use in new code. */ +void drm_fb_helper_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) +{ + struct drm_fb_helper *fb_helper = info->par; + + drm_fb_helper_damage(fb_helper, x, y, width, height); +} +EXPORT_SYMBOL(drm_fb_helper_damage_area); + /** * drm_fb_helper_deferred_io() - fbdev deferred_io callback function * @info: fb_info struct pointer diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 72032c354a30..7d5804882be7 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -253,6 +253,9 @@ void drm_fb_helper_fill_info(struct fb_info *info, struct drm_fb_helper *fb_helper, struct drm_fb_helper_surface_size *sizes); +void drm_fb_helper_damage_range(struct fb_info *info, off_t off, size_t len); +void drm_fb_helper_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height); + void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagereflist); ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, -- 2.40.1
[Freedreno] [PATCH v4 07/13] drm/fbdev-dma: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Fbdev-dma does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. v4: * use initializer macros for struct fb_ops v2: * use FB_SYS_HELPERS option Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_fbdev_dma.c | 11 +-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index ba3fb04bb691..77fb10ddd8a2 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -223,6 +223,7 @@ config DRM_TTM_HELPER config DRM_GEM_DMA_HELPER tristate depends on DRM + select FB_SYS_HELPERS if DRM_FBDEV_EMULATION help Choose this if you need the GEM DMA helper functions diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c index 728deffcc0d9..d86773fa8ab0 100644 --- a/drivers/gpu/drm/drm_fbdev_dma.c +++ b/drivers/gpu/drm/drm_fbdev_dma.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: MIT +#include + #include #include #include @@ -64,14 +66,11 @@ static const struct fb_ops drm_fbdev_dma_fb_ops = { .owner = THIS_MODULE, .fb_open = drm_fbdev_dma_fb_open, .fb_release = drm_fbdev_dma_fb_release, - .fb_read = drm_fb_helper_sys_read, - .fb_write = drm_fb_helper_sys_write, + __FB_DEFAULT_SYS_OPS_RDWR, DRM_FB_HELPER_DEFAULT_OPS, - .fb_fillrect = drm_fb_helper_sys_fillrect, - .fb_copyarea = drm_fb_helper_sys_copyarea, - .fb_imageblit = drm_fb_helper_sys_imageblit, - .fb_destroy = drm_fbdev_dma_fb_destroy, + __FB_DEFAULT_SYS_OPS_DRAW, .fb_mmap = drm_fbdev_dma_fb_mmap, + .fb_destroy = drm_fbdev_dma_fb_destroy, }; /* -- 2.40.1
[Freedreno] [PATCH v4 09/13] drm/omapdrm: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Omapdrm does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. v4: * use initializer macros for struct fb_ops v2: * use FB_SYS_HELPERS option Signed-off-by: Thomas Zimmermann Cc: Tomi Valkeinen --- drivers/gpu/drm/omapdrm/Kconfig | 1 + drivers/gpu/drm/omapdrm/omap_fbdev.c | 11 +++ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/Kconfig b/drivers/gpu/drm/omapdrm/Kconfig index 76ded1568bd0..b4ac76c9f31b 100644 --- a/drivers/gpu/drm/omapdrm/Kconfig +++ b/drivers/gpu/drm/omapdrm/Kconfig @@ -4,6 +4,7 @@ config DRM_OMAP depends on DRM && OF depends on ARCH_OMAP2PLUS select DRM_KMS_HELPER + select FB_SYS_HELPERS if DRM_FBDEV_EMULATION select VIDEOMODE_HELPERS select HDMI default n diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c index b950e93b3846..b7ccce0704a3 100644 --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c @@ -4,6 +4,8 @@ * Author: Rob Clark */ +#include + #include #include #include @@ -95,20 +97,13 @@ static void omap_fbdev_fb_destroy(struct fb_info *info) static const struct fb_ops omap_fb_ops = { .owner = THIS_MODULE, - + FB_DEFAULT_SYS_OPS, .fb_check_var = drm_fb_helper_check_var, .fb_set_par = drm_fb_helper_set_par, .fb_setcmap = drm_fb_helper_setcmap, .fb_blank = drm_fb_helper_blank, .fb_pan_display = omap_fbdev_pan_display, .fb_ioctl = drm_fb_helper_ioctl, - - .fb_read = drm_fb_helper_sys_read, - .fb_write = drm_fb_helper_sys_write, - .fb_fillrect = drm_fb_helper_sys_fillrect, - .fb_copyarea = drm_fb_helper_sys_copyarea, - .fb_imageblit = drm_fb_helper_sys_imageblit, - .fb_destroy = omap_fbdev_fb_destroy, }; -- 2.40.1
[Freedreno] [PATCH v4 10/13] drm/tegra: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Tegra does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. v4: * use initializer macros for struct fb_ops v2: * use FB_SYS_HELPERS option Signed-off-by: Thomas Zimmermann Cc: Thierry Reding Cc: Mikko Perttunen Cc: Jonathan Hunter --- drivers/gpu/drm/tegra/Kconfig | 1 + drivers/gpu/drm/tegra/fbdev.c | 8 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig index 56453ca277c2..498313778175 100644 --- a/drivers/gpu/drm/tegra/Kconfig +++ b/drivers/gpu/drm/tegra/Kconfig @@ -12,6 +12,7 @@ config DRM_TEGRA select DRM_KMS_HELPER select DRM_MIPI_DSI select DRM_PANEL + select FB_SYS_HELPERS if DRM_FBDEV_EMULATION select TEGRA_HOST1X select INTERCONNECT select IOMMU_IOVA diff --git a/drivers/gpu/drm/tegra/fbdev.c b/drivers/gpu/drm/tegra/fbdev.c index dca9eccae466..e74d9be981c7 100644 --- a/drivers/gpu/drm/tegra/fbdev.c +++ b/drivers/gpu/drm/tegra/fbdev.c @@ -8,6 +8,7 @@ */ #include +#include #include #include @@ -58,12 +59,9 @@ static void tegra_fbdev_fb_destroy(struct fb_info *info) static const struct fb_ops tegra_fb_ops = { .owner = THIS_MODULE, + __FB_DEFAULT_SYS_OPS_RDWR, DRM_FB_HELPER_DEFAULT_OPS, - .fb_read = drm_fb_helper_sys_read, - .fb_write = drm_fb_helper_sys_write, - .fb_fillrect = drm_fb_helper_sys_fillrect, - .fb_copyarea = drm_fb_helper_sys_copyarea, - .fb_imageblit = drm_fb_helper_sys_imageblit, + __FB_DEFAULT_SYS_OPS_DRAW, .fb_mmap = tegra_fb_mmap, .fb_destroy = tegra_fbdev_fb_destroy, }; -- 2.40.1
[Freedreno] [PATCH v4 12/13] drm/fbdev-generic: Implement dedicated fbdev I/O helpers
Implement dedicated fbdev helpers for framebuffer I/O instead of using DRM's helpers. Use an fbdev generator macro for deferred I/O to create the callbacks. Fbdev-generic was the only caller of the DRM helpers, so remove them from the helper module. v4: * generate deferred-I/O helpers * use initializer macros for fb_ops v2: * use FB_SYS_HELPERS_DEFERRED option Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/Kconfig | 6 +- drivers/gpu/drm/drm_fb_helper.c | 107 drivers/gpu/drm/drm_fbdev_generic.c | 11 ++- include/drm/drm_fb_helper.h | 41 --- 4 files changed, 6 insertions(+), 159 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 77fb10ddd8a2..92a782827b7b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -95,6 +95,7 @@ config DRM_KUNIT_TEST config DRM_KMS_HELPER tristate depends on DRM + select FB_SYS_HELPERS_DEFERRED if DRM_FBDEV_EMULATION help CRTC helpers for KMS drivers. @@ -135,11 +136,6 @@ config DRM_FBDEV_EMULATION select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT - select FB_DEFERRED_IO - select FB_SYS_FOPS - select FB_SYS_FILLRECT - select FB_SYS_COPYAREA - select FB_SYS_IMAGEBLIT select FRAMEBUFFER_CONSOLE if !EXPERT select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE default y diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index cb03099fd2e3..bab6b252f02a 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -736,113 +736,6 @@ void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagerefli } EXPORT_SYMBOL(drm_fb_helper_deferred_io); -/** - * drm_fb_helper_sys_read - Implements struct _ops.fb_read for system memory - * @info: fb_info struct pointer - * @buf: userspace buffer to read from framebuffer memory - * @count: number of bytes to read from framebuffer memory - * @ppos: read offset within framebuffer memory - * - * Returns: - * The number of bytes read on success, or an error code otherwise. - */ -ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, - size_t count, loff_t *ppos) -{ - return fb_sys_read(info, buf, count, ppos); -} -EXPORT_SYMBOL(drm_fb_helper_sys_read); - -/** - * drm_fb_helper_sys_write - Implements struct _ops.fb_write for system memory - * @info: fb_info struct pointer - * @buf: userspace buffer to write to framebuffer memory - * @count: number of bytes to write to framebuffer memory - * @ppos: write offset within framebuffer memory - * - * Returns: - * The number of bytes written on success, or an error code otherwise. - */ -ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, - size_t count, loff_t *ppos) -{ - struct drm_fb_helper *helper = info->par; - loff_t pos = *ppos; - ssize_t ret; - struct drm_rect damage_area; - - ret = fb_sys_write(info, buf, count, ppos); - if (ret <= 0) - return ret; - - if (helper->funcs->fb_dirty) { - drm_fb_helper_memory_range_to_clip(info, pos, ret, _area); - drm_fb_helper_damage(helper, damage_area.x1, damage_area.y1, -drm_rect_width(_area), -drm_rect_height(_area)); - } - - return ret; -} -EXPORT_SYMBOL(drm_fb_helper_sys_write); - -/** - * drm_fb_helper_sys_fillrect - wrapper around sys_fillrect - * @info: fbdev registered by the helper - * @rect: info about rectangle to fill - * - * A wrapper around sys_fillrect implemented by fbdev core - */ -void drm_fb_helper_sys_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct drm_fb_helper *helper = info->par; - - sys_fillrect(info, rect); - - if (helper->funcs->fb_dirty) - drm_fb_helper_damage(helper, rect->dx, rect->dy, rect->width, rect->height); -} -EXPORT_SYMBOL(drm_fb_helper_sys_fillrect); - -/** - * drm_fb_helper_sys_copyarea - wrapper around sys_copyarea - * @info: fbdev registered by the helper - * @area: info about area to copy - * - * A wrapper around sys_copyarea implemented by fbdev core - */ -void drm_fb_helper_sys_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - struct drm_fb_helper *helper = info->par; - - sys_copyarea(info, area); - - if (helper->funcs->fb_dirty) - drm_fb_helper_damage(helper, area->dx, area->dy, area->width, area->height); -} -EXPORT_SYMBOL(drm_fb_helper_sys_copyarea); - -/** - * drm_fb_helper_sys_imageblit - wrapper around sys_imageblit - * @info: fbdev registered by the helper - * @image: info about image to blit - * - * A wrapper
[Freedreno] [PATCH v4 08/13] drm/msm: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Msm does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. Msm's fbdev emulation has been incomplete as it didn't implement damage handling. Partilly fix this by implementing damage handling for write and draw operation. It is still missing for mmaped pages. v4: * use initializer macros for struct fb_ops * partially support damage handling v2: * use FB_SYS_HELPERS option Signed-off-by: Thomas Zimmermann Reviewed-by: Dmitry Baryshkov Cc: Rob Clark Cc: Abhinav Kumar Cc: Dmitry Baryshkov Cc: Sean Paul --- drivers/gpu/drm/msm/Kconfig | 1 + drivers/gpu/drm/msm/msm_fbdev.c | 17 - 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 85f5ab1d552c..a78662bd6273 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -21,6 +21,7 @@ config DRM_MSM select DRM_BRIDGE select DRM_PANEL_BRIDGE select DRM_SCHED + select FB_SYS_HELPERS if DRM_FBDEV_EMULATION select SHMEM select TMPFS select QCOM_SCM diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c index ce0ba6d1979a..fa9c1cbffae3 100644 --- a/drivers/gpu/drm/msm/msm_fbdev.c +++ b/drivers/gpu/drm/msm/msm_fbdev.c @@ -4,6 +4,8 @@ * Author: Rob Clark */ +#include + #include #include #include @@ -23,6 +25,10 @@ module_param(fbdev, bool, 0600); * fbdev funcs, to implement legacy fbdev interface on top of drm driver */ +FB_GEN_DEFAULT_DEFERRED_SYS_OPS(msm_fbdev, + drm_fb_helper_damage_range, + drm_fb_helper_damage_area) + static int msm_fbdev_mmap(struct fb_info *info, struct vm_area_struct *vma) { struct drm_fb_helper *helper = (struct drm_fb_helper *)info->par; @@ -52,16 +58,9 @@ static void msm_fbdev_fb_destroy(struct fb_info *info) static const struct fb_ops msm_fb_ops = { .owner = THIS_MODULE, + __FB_DEFAULT_DEFERRED_OPS_RDWR(msm_fbdev), DRM_FB_HELPER_DEFAULT_OPS, - - /* Note: to properly handle manual update displays, we wrap the -* basic fbdev ops which write to the framebuffer -*/ - .fb_read = drm_fb_helper_sys_read, - .fb_write = drm_fb_helper_sys_write, - .fb_fillrect = drm_fb_helper_sys_fillrect, - .fb_copyarea = drm_fb_helper_sys_copyarea, - .fb_imageblit = drm_fb_helper_sys_imageblit, + __FB_DEFAULT_DEFERRED_OPS_DRAW(msm_fbdev), .fb_mmap = msm_fbdev_mmap, .fb_destroy = msm_fbdev_fb_destroy, }; -- 2.40.1
[Freedreno] [PATCH v4 00/13] drm/fbdev: Remove DRM's helpers for fbdev I/O
DRM provides a number of wrappers around fbdev cfb_() sys_(), fb_io_() and fb_sys_() helpers. The DRM functions don't provide any additional functionality for most DRM drivers. So remove them and call the fbdev I/O helpers directly. The DRM fbdev I/O wrappers were originally added because does not protect its content with CONFIG_FB. DRM fbdev emulation did not build if the config option had been disabled. This has been fixed. For fbdev-generic and i915, the wrappers added support for damage handling. But this is better handled within the two callers, as each is special in its damage handling. Patch 1 adds several internal Kconfig options that DRM drivers (and possibly other fbdev code) will use to select the correct set of I/O helpers. Patch 2 adds initializers for struct fb_ops and generator macros for the callback functions. These macros will simplify drivers. This patchset applies the new options and macros to DRM fbdev emulation, but regular fbdev drivers can use them as well. Patches 3 to 10 replace the DRM wrappers in a number of fbdev emulations. Patch 10 exports two helpers for damage handling. Patches 12 and 13 update fbdev-generic and i915 with the help of the exported functions. The patches also remove DRM's fbdev I/O helpers, which are now unused. DRM's fbdev helpers had to select fbdev I/O helpers for I/O and for system memory. Each fbdev emulation now selects the correct helpers for itself. Depending on the selected DRM drivers, kernel builds will now only contain the necessary fbdev I/O helpers and might be slightly smaller in size. v4: * use initializer and generator macros for struct fb_ops * partially support damage handling in msm (Dmitri) v3: * fix Kconfig options (Jingfeng) * minimize changes to exynos (Sam) v2: * simplify Kconfig handling (Sam) Thomas Zimmermann (13): fbdev: Add Kconfig options to select different fb_ops helpers fbdev: Add initializer macros for struct fb_ops drm/armada: Use regular fbdev I/O helpers drm/exynos: Use regular fbdev I/O helpers drm/gma500: Use regular fbdev I/O helpers drm/radeon: Use regular fbdev I/O helpers drm/fbdev-dma: Use regular fbdev I/O helpers drm/msm: Use regular fbdev I/O helpers drm/omapdrm: Use regular fbdev I/O helpers drm/tegra: Use regular fbdev I/O helpers drm/fb-helper: Export helpers for marking damage areas drm/fbdev-generic: Implement dedicated fbdev I/O helpers drm/i915: Implement dedicated fbdev I/O helpers drivers/gpu/drm/Kconfig| 10 +- drivers/gpu/drm/armada/Kconfig | 1 + drivers/gpu/drm/armada/armada_fbdev.c | 7 +- drivers/gpu/drm/drm_fb_helper.c| 236 ++--- drivers/gpu/drm/drm_fbdev_dma.c| 11 +- drivers/gpu/drm/drm_fbdev_generic.c| 11 +- drivers/gpu/drm/exynos/Kconfig | 1 + drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 9 +- drivers/gpu/drm/gma500/Kconfig | 1 + drivers/gpu/drm/gma500/fbdev.c | 8 +- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/display/intel_fbdev.c | 14 +- drivers/gpu/drm/msm/Kconfig| 1 + drivers/gpu/drm/msm/msm_fbdev.c| 17 +- drivers/gpu/drm/omapdrm/Kconfig| 1 + drivers/gpu/drm/omapdrm/omap_fbdev.c | 11 +- drivers/gpu/drm/radeon/Kconfig | 1 + drivers/gpu/drm/radeon/radeon_fbdev.c | 9 +- drivers/gpu/drm/tegra/Kconfig | 1 + drivers/gpu/drm/tegra/fbdev.c | 8 +- drivers/video/fbdev/Kconfig| 21 ++ include/drm/drm_fb_helper.h| 83 +--- include/linux/fb.h | 112 ++ 23 files changed, 212 insertions(+), 363 deletions(-) base-commit: 216281f91018b24567e59ae46ce7e96fb92063cf prerequisite-patch-id: 0aa359f6144c4015c140c8a6750be19099c676fb prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 prerequisite-patch-id: cbc453ee02fae02af22fbfdce56ab732c7a88c36 prerequisite-patch-id: 8bff2b12862e44027a25837ea7510f633d40839e prerequisite-patch-id: 97ac107455aff4e0ec039d166ecdd2430d20f22e -- 2.40.1
[Freedreno] [PATCH v4 13/13] drm/i915: Implement dedicated fbdev I/O helpers
Implement dedicated fbdev helpers for framebuffer I/O instead of using DRM's helpers. Use an fbdev generator macro for deferred I/O to create the fbdev callbacks. i915 was the only caller of the DRM helpers, so remove them from the helper module. i915's fbdev emulation is still incomplete as it doesn't implement deferred I/O and damage handling for mmaped pages. v4: * generate deferred-I/O helpers * use initializer macros for fb_ops v2: * use FB_IO_HELPERS options Signed-off-by: Thomas Zimmermann Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Tvrtko Ursulin Cc: "Ville Syrjälä" --- drivers/gpu/drm/Kconfig| 3 - drivers/gpu/drm/drm_fb_helper.c| 107 - drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/display/intel_fbdev.c | 14 +-- include/drm/drm_fb_helper.h| 39 5 files changed, 9 insertions(+), 155 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 92a782827b7b..bb2e48cc6cd6 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -133,9 +133,6 @@ config DRM_FBDEV_EMULATION bool "Enable legacy fbdev support for your modesetting driver" depends on DRM_KMS_HELPER depends on FB=y || FB=DRM_KMS_HELPER - select FB_CFB_FILLRECT - select FB_CFB_COPYAREA - select FB_CFB_IMAGEBLIT select FRAMEBUFFER_CONSOLE if !EXPERT select FRAMEBUFFER_CONSOLE_DETECT_PRIMARY if FRAMEBUFFER_CONSOLE default y diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index bab6b252f02a..9978147bbc8a 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -736,113 +736,6 @@ void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagerefli } EXPORT_SYMBOL(drm_fb_helper_deferred_io); -/** - * drm_fb_helper_cfb_read - Implements struct _ops.fb_read for I/O memory - * @info: fb_info struct pointer - * @buf: userspace buffer to read from framebuffer memory - * @count: number of bytes to read from framebuffer memory - * @ppos: read offset within framebuffer memory - * - * Returns: - * The number of bytes read on success, or an error code otherwise. - */ -ssize_t drm_fb_helper_cfb_read(struct fb_info *info, char __user *buf, - size_t count, loff_t *ppos) -{ - return fb_io_read(info, buf, count, ppos); -} -EXPORT_SYMBOL(drm_fb_helper_cfb_read); - -/** - * drm_fb_helper_cfb_write - Implements struct _ops.fb_write for I/O memory - * @info: fb_info struct pointer - * @buf: userspace buffer to write to framebuffer memory - * @count: number of bytes to write to framebuffer memory - * @ppos: write offset within framebuffer memory - * - * Returns: - * The number of bytes written on success, or an error code otherwise. - */ -ssize_t drm_fb_helper_cfb_write(struct fb_info *info, const char __user *buf, - size_t count, loff_t *ppos) -{ - struct drm_fb_helper *helper = info->par; - loff_t pos = *ppos; - ssize_t ret; - struct drm_rect damage_area; - - ret = fb_io_write(info, buf, count, ppos); - if (ret <= 0) - return ret; - - if (helper->funcs->fb_dirty) { - drm_fb_helper_memory_range_to_clip(info, pos, ret, _area); - drm_fb_helper_damage(helper, damage_area.x1, damage_area.y1, -drm_rect_width(_area), -drm_rect_height(_area)); - } - - return ret; -} -EXPORT_SYMBOL(drm_fb_helper_cfb_write); - -/** - * drm_fb_helper_cfb_fillrect - wrapper around cfb_fillrect - * @info: fbdev registered by the helper - * @rect: info about rectangle to fill - * - * A wrapper around cfb_fillrect implemented by fbdev core - */ -void drm_fb_helper_cfb_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct drm_fb_helper *helper = info->par; - - cfb_fillrect(info, rect); - - if (helper->funcs->fb_dirty) - drm_fb_helper_damage(helper, rect->dx, rect->dy, rect->width, rect->height); -} -EXPORT_SYMBOL(drm_fb_helper_cfb_fillrect); - -/** - * drm_fb_helper_cfb_copyarea - wrapper around cfb_copyarea - * @info: fbdev registered by the helper - * @area: info about area to copy - * - * A wrapper around cfb_copyarea implemented by fbdev core - */ -void drm_fb_helper_cfb_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - struct drm_fb_helper *helper = info->par; - - cfb_copyarea(info, area); - - if (helper->funcs->fb_dirty) - drm_fb_helper_damage(helper, area->dx, area->dy, area->width, area->height); -} -EXPORT_SYMBOL(drm_fb_helper_cfb_copyarea); - -/** - * drm_fb_helper_cfb_imageblit - wrapper around cfb_imageblit - * @info: fbdev registered by the helper - * @image:
[Freedreno] [PATCH v4 03/13] drm/armada: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Armada does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. v4: * use initializer macros for struct fb_ops v2: * use FB_IO_HELPERS option Signed-off-by: Thomas Zimmermann Cc: Russell King --- drivers/gpu/drm/armada/Kconfig| 1 + drivers/gpu/drm/armada/armada_fbdev.c | 7 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/armada/Kconfig b/drivers/gpu/drm/armada/Kconfig index f5c66d89ba99..5afade25e217 100644 --- a/drivers/gpu/drm/armada/Kconfig +++ b/drivers/gpu/drm/armada/Kconfig @@ -3,6 +3,7 @@ config DRM_ARMADA tristate "DRM support for Marvell Armada SoCs" depends on DRM && HAVE_CLK && ARM && MMU select DRM_KMS_HELPER + select FB_IO_HELPERS if DRM_FBDEV_EMULATION help Support the "LCD" controllers found on the Marvell Armada 510 devices. There are two controllers on the device, each controller diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c index 0a5fd1aa86eb..3943e89cc06c 100644 --- a/drivers/gpu/drm/armada/armada_fbdev.c +++ b/drivers/gpu/drm/armada/armada_fbdev.c @@ -5,6 +5,7 @@ */ #include +#include #include #include @@ -33,12 +34,8 @@ static void armada_fbdev_fb_destroy(struct fb_info *info) static const struct fb_ops armada_fb_ops = { .owner = THIS_MODULE, + FB_DEFAULT_IO_OPS, DRM_FB_HELPER_DEFAULT_OPS, - .fb_read= drm_fb_helper_cfb_read, - .fb_write = drm_fb_helper_cfb_write, - .fb_fillrect= drm_fb_helper_cfb_fillrect, - .fb_copyarea= drm_fb_helper_cfb_copyarea, - .fb_imageblit = drm_fb_helper_cfb_imageblit, .fb_destroy = armada_fbdev_fb_destroy, }; -- 2.40.1
[Freedreno] [PATCH v4 06/13] drm/radeon: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Radeon does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. v4: * use initializer macros for struct fb_ops v2: * use FB_IO_HELPERS option Signed-off-by: Thomas Zimmermann Acked-by: Alex Deucher Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" --- drivers/gpu/drm/radeon/Kconfig| 1 + drivers/gpu/drm/radeon/radeon_fbdev.c | 9 +++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig index e19d77d58810..fe498c8af1bb 100644 --- a/drivers/gpu/drm/radeon/Kconfig +++ b/drivers/gpu/drm/radeon/Kconfig @@ -11,6 +11,7 @@ config DRM_RADEON select DRM_SUBALLOC_HELPER select DRM_TTM select DRM_TTM_HELPER + select FB_IO_HELPERS if DRM_FBDEV_EMULATION select SND_HDA_COMPONENT if SND_HDA_CORE select POWER_SUPPLY select HWMON diff --git a/drivers/gpu/drm/radeon/radeon_fbdev.c b/drivers/gpu/drm/radeon/radeon_fbdev.c index fe76e29910ef..28212c2d6c98 100644 --- a/drivers/gpu/drm/radeon/radeon_fbdev.c +++ b/drivers/gpu/drm/radeon/radeon_fbdev.c @@ -24,6 +24,7 @@ * David Airlie */ +#include #include #include #include @@ -190,14 +191,10 @@ static void radeon_fbdev_fb_destroy(struct fb_info *info) static const struct fb_ops radeon_fbdev_fb_ops = { .owner = THIS_MODULE, - DRM_FB_HELPER_DEFAULT_OPS, .fb_open = radeon_fbdev_fb_open, .fb_release = radeon_fbdev_fb_release, - .fb_read = drm_fb_helper_cfb_read, - .fb_write = drm_fb_helper_cfb_write, - .fb_fillrect = drm_fb_helper_cfb_fillrect, - .fb_copyarea = drm_fb_helper_cfb_copyarea, - .fb_imageblit = drm_fb_helper_cfb_imageblit, + FB_DEFAULT_IO_OPS, + DRM_FB_HELPER_DEFAULT_OPS, .fb_destroy = radeon_fbdev_fb_destroy, }; -- 2.40.1
[Freedreno] [PATCH v4 02/13] fbdev: Add initializer macros for struct fb_ops
For framebuffers in I/O and system memory, add macros that set struct fb_ops to the respective callback functions. For deferred I/O, add macros that generate callback functions with damage handling. Add initializer macros that set struct fb_ops to the generated callbacks. These macros can remove a lot boilerplate code from fbdev drivers. The drivers are supposed to use the macro that is required for its framebuffer. Each macro is split into smaller helpers, so that drivers with non-standard callbacks can pick and customize callbacks as needed. There are individual helper macros for read/write, mmap and drawing. Signed-off-by: Thomas Zimmermann --- include/linux/fb.h | 112 + 1 file changed, 112 insertions(+) diff --git a/include/linux/fb.h b/include/linux/fb.h index 2cf8efcb9e32..731472a2bb62 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -538,9 +538,31 @@ extern ssize_t fb_io_read(struct fb_info *info, char __user *buf, extern ssize_t fb_io_write(struct fb_info *info, const char __user *buf, size_t count, loff_t *ppos); +/* + * Initializes struct fb_ops for framebuffers in I/O memory. + */ + +#define __FB_DEFAULT_IO_OPS_RDWR \ + .fb_read= fb_io_read, \ + .fb_write = fb_io_write + +#define __FB_DEFAULT_IO_OPS_DRAW \ +.fb_fillrect = cfb_fillrect, \ +.fb_copyarea = cfb_copyarea, \ +.fb_imageblit = cfb_imageblit + +#define __FB_DEFAULT_IO_OPS_MMAP \ + .fb_mmap= NULL // default implementation + +#define FB_DEFAULT_IO_OPS \ + __FB_DEFAULT_IO_OPS_RDWR, \ + __FB_DEFAULT_IO_OPS_DRAW, \ + __FB_DEFAULT_IO_OPS_MMAP + /* * Drawing operations where framebuffer is in system RAM */ + extern void sys_fillrect(struct fb_info *info, const struct fb_fillrect *rect); extern void sys_copyarea(struct fb_info *info, const struct fb_copyarea *area); extern void sys_imageblit(struct fb_info *info, const struct fb_image *image); @@ -549,6 +571,27 @@ extern ssize_t fb_sys_read(struct fb_info *info, char __user *buf, extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf, size_t count, loff_t *ppos); +/* + * Initializes struct fb_ops for framebuffers in system memory. + */ + +#define __FB_DEFAULT_SYS_OPS_RDWR \ + .fb_read= fb_sys_read, \ + .fb_write = fb_sys_write + +#define __FB_DEFAULT_SYS_OPS_DRAW \ +.fb_fillrect = sys_fillrect, \ +.fb_copyarea = sys_copyarea, \ +.fb_imageblit = sys_imageblit + +#define __FB_DEFAULT_SYS_OPS_MMAP \ + .fb_mmap= NULL // default implementation + +#define FB_DEFAULT_SYS_OPS \ + __FB_DEFAULT_SYS_OPS_RDWR, \ + __FB_DEFAULT_SYS_OPS_DRAW, \ + __FB_DEFAULT_SYS_OPS_MMAP + /* drivers/video/fbmem.c */ extern int register_framebuffer(struct fb_info *fb_info); extern void unregister_framebuffer(struct fb_info *fb_info); @@ -604,6 +647,75 @@ extern void fb_deferred_io_cleanup(struct fb_info *info); extern int fb_deferred_io_fsync(struct file *file, loff_t start, loff_t end, int datasync); +/* + * Generate callbacks for deferred I/O + */ + +#define __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, __mode) \ + static ssize_t __prefix ## _defio_read(struct fb_info *info, char __user *buf, \ + size_t count, loff_t *ppos) \ + { \ + return fb_ ## __mode ## _read(info, buf, count, ppos); \ + } \ + static ssize_t __prefix ## _defio_write(struct fb_info *info, const char __user *buf, \ + size_t count, loff_t *ppos) \ + { \ + unsigned long offset = *ppos; \ + ssize_t ret = fb_ ## __mode ## _write(info, buf, count, ppos); \ + if (ret > 0) \ + __damage_range(info, offset, ret); \ + return ret; \ + } + +#define __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, __mode) \ + static void __prefix ## _defio_fillrect(struct fb_info *info, \ + const struct fb_fillrect *rect) \ + { \ + __mode ## _fillrect(info, rect); \ + __damage_area(info, rect->dx, rect->dy, rect->width, rect->height); \ + } \ + static void __prefix ## _defio_copyarea(struct fb_info *info, \ + const struct fb_copyarea *area) \ + { \ + __mode ## _copyarea(info, area); \ + __damage_area(info, area->dx, area->dy, area->width, area->height); \ + } \ + static void __prefix ## _defio_imageblit(struct fb_info *info, \ +const struct fb_image *image) \ + { \ + __mode ## _imageblit(info, image); \ +
[Freedreno] [PATCH v4 04/13] drm/exynos: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Exynos does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. v4: * use initializer macros for struct fb_ops v3: * don't reorder Makefile rules (Sam) v2: * use FB_IO_HELPERS option Signed-off-by: Thomas Zimmermann Cc: Inki Dae Cc: Seung-Woo Kim Cc: Kyungmin Park Cc: Krzysztof Kozlowski Cc: Alim Akhtar --- drivers/gpu/drm/exynos/Kconfig| 1 + drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 9 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 0cb92d651ff1..7ca7e1dab52c 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -7,6 +7,7 @@ config DRM_EXYNOS select DRM_DISPLAY_HELPER if DRM_EXYNOS_DP select DRM_KMS_HELPER select VIDEOMODE_HELPERS + select FB_IO_HELPERS if DRM_FBDEV_EMULATION select SND_SOC_HDMI_CODEC if SND_SOC help Choose this option if you have a Samsung SoC Exynos chipset. diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index ea4b3d248aac..fdf65587f1fe 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -8,6 +8,8 @@ * Seung-Woo Kim */ +#include + #include #include #include @@ -47,13 +49,10 @@ static void exynos_drm_fb_destroy(struct fb_info *info) static const struct fb_ops exynos_drm_fb_ops = { .owner = THIS_MODULE, + __FB_DEFAULT_IO_OPS_RDWR, DRM_FB_HELPER_DEFAULT_OPS, + __FB_DEFAULT_IO_OPS_DRAW, .fb_mmap= exynos_drm_fb_mmap, - .fb_read= drm_fb_helper_cfb_read, - .fb_write = drm_fb_helper_cfb_write, - .fb_fillrect= drm_fb_helper_cfb_fillrect, - .fb_copyarea= drm_fb_helper_cfb_copyarea, - .fb_imageblit = drm_fb_helper_cfb_imageblit, .fb_destroy = exynos_drm_fb_destroy, }; -- 2.40.1
[Freedreno] [PATCH v4 05/13] drm/gma500: Use regular fbdev I/O helpers
Use the regular fbdev helpers for framebuffer I/O instead of DRM's helpers. Gma500 does not use damage handling, so DRM's fbdev helpers are mere wrappers around the fbdev code. By using fbdev helpers directly within each DRM fbdev emulation, we can eventually remove DRM's wrapper functions entirely. v4: * use initializer macros for struct fb_ops v2: * use FB_IO_HELPERS option Signed-off-by: Thomas Zimmermann Cc: Patrik Jakobsson --- drivers/gpu/drm/gma500/Kconfig | 1 + drivers/gpu/drm/gma500/fbdev.c | 8 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig index 2efc0eb41c64..cd3d92725ed4 100644 --- a/drivers/gpu/drm/gma500/Kconfig +++ b/drivers/gpu/drm/gma500/Kconfig @@ -3,6 +3,7 @@ config DRM_GMA500 tristate "Intel GMA500/600/3600/3650 KMS Framebuffer" depends on DRM && PCI && X86 && MMU select DRM_KMS_HELPER + select FB_IO_HELPERS if DRM_FBDEV_EMULATION select I2C select I2C_ALGOBIT # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915 diff --git a/drivers/gpu/drm/gma500/fbdev.c b/drivers/gpu/drm/gma500/fbdev.c index 4f0309548b2b..955cbe9f05a7 100644 --- a/drivers/gpu/drm/gma500/fbdev.c +++ b/drivers/gpu/drm/gma500/fbdev.c @@ -5,6 +5,7 @@ * **/ +#include #include #include @@ -134,13 +135,10 @@ static void psb_fbdev_fb_destroy(struct fb_info *info) static const struct fb_ops psb_fbdev_fb_ops = { .owner = THIS_MODULE, + __FB_DEFAULT_IO_OPS_RDWR, DRM_FB_HELPER_DEFAULT_OPS, .fb_setcolreg = psb_fbdev_fb_setcolreg, - .fb_read = drm_fb_helper_cfb_read, - .fb_write = drm_fb_helper_cfb_write, - .fb_fillrect = drm_fb_helper_cfb_fillrect, - .fb_copyarea = drm_fb_helper_cfb_copyarea, - .fb_imageblit = drm_fb_helper_cfb_imageblit, + __FB_DEFAULT_IO_OPS_DRAW, .fb_mmap = psb_fbdev_fb_mmap, .fb_destroy = psb_fbdev_fb_destroy, }; -- 2.40.1
[Freedreno] [PATCH v4 01/13] fbdev: Add Kconfig options to select different fb_ops helpers
Many fbdev drivers use the same set of fb_ops helpers. Add Kconfig options to select them at once. This will help with making DRM's fbdev emulation code more modular, but can also be used to simplify fbdev's driver configs. v3: * fix select statement (Jingfeng) Signed-off-by: Thomas Zimmermann --- drivers/video/fbdev/Kconfig | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index e8889035c882..6df9bd09454a 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -158,6 +158,27 @@ config FB_DEFERRED_IO bool depends on FB +config FB_IO_HELPERS + bool + depends on FB + select FB_CFB_COPYAREA + select FB_CFB_FILLRECT + select FB_CFB_IMAGEBLIT + +config FB_SYS_HELPERS + bool + depends on FB + select FB_SYS_COPYAREA + select FB_SYS_FILLRECT + select FB_SYS_FOPS + select FB_SYS_IMAGEBLIT + +config FB_SYS_HELPERS_DEFERRED + bool + depends on FB + select FB_DEFERRED_IO + select FB_SYS_HELPERS + config FB_HECUBA tristate depends on FB -- 2.40.1
Re: [Freedreno] [PATCH] drm/msm/dp: add module parameter for PSR
On 24/05/2023 09:59, Johan Hovold wrote: On Tue, May 23, 2023 at 12:23:04PM -0700, Abhinav Kumar wrote: On 5/23/2023 8:24 AM, Johan Hovold wrote: On Fri, May 12, 2023 at 09:13:04PM +0300, Dmitry Baryshkov wrote: On 28/04/2023 02:28, Abhinav Kumar wrote: On sc7280 where eDP is the primary display, PSR is causing IGT breakage even for basic test cases like kms_atomic and kms_atomic_transition. Most often the issue starts with below stack so providing that as reference Call trace: ---[ end trace ]--- [drm-dp] dp_ctrl_push_idle: PUSH_IDLE pattern timedout Other basic use-cases still seem to work fine hence add a a module parameter to allow toggling psr enable/disable till PSR related issues are hashed out with IGT. For the reference: Bjorn reported that he has issues with VT on a PSR-enabled laptops. This patch fixes the issue for him Module parameters are almost never warranted, and it is definitely not the right way to handle a broken implementation. I've just sent a revert that unconditionally disables PSR support until the implementation has been fixed: https://lore.kernel.org/lkml/20230523151646.28366-1-johan+lin...@kernel.org/ I dont completely agree with this. Even the virtual terminal case was reported to be fixed by one user but not the other. So it was probably something missed out either in validation or reproduction steps of the user who reported it to be fixed OR the user who reported it not fixed. That needs to be investigated now. Yes, there may still be some time left to fix it, but it's pretty damn annoying to find that an issue reported two months ago still is not fixed at 6.4-rc3. (I even waited to make the switch to 6.4 so that I would not have to spend time on this.) I didn't see any mail from Bjorn saying that the series that claimed to fix the VT issue actually did fix the VT issue. There's only the comment above from Dmitry suggesting that disabling this feature is the only way to get a working terminal back. Originally this issue was reported by Doug, and at [1] he reported that an issue is fixed for him. So, for me it looks like we have hardware where VT works and hardware where it doesn't. Doug, can you please confirm whether you can reproduce the PSR+VT issue on 6.4-rc (without extra patches) or if the issue is fixed for you? [1] https://lore.kernel.org/dri-devel/CAD=FV=vshmqptsqfwjviezeerms-veotmfozejasuc9zsmj...@mail.gmail.com/ Regressions happen and sometimes there are corner cases that are harder to find, but this is a breakage of a fundamental feature that was reported before the code was even merged into mainline. We should have ideally gone with the modparam with the feature patches itself knowing that it gets enabled for all sinks if PSR is supported. Modparams are things of the past should not be used to enable broken features so that some vendor can tick of their internal lists of features that have been "mainlined". We have had a history of using modparam with i915 and IIRC amdgpu / radeon drivers to allow users to easily check whether new feature works for their hardware. My current understanding is that PSR+VT works for on some laptops and doesn't on some other laptops, which makes it a valid case. You can carry that single patch out-of-tree to enable this if you need it for some particular use case where you don't care about VTs. But hopefully you can just get this sorted quickly. If not, the revert I posted is the way to go rather than adding random module parameters. Johan -- With best wishes Dmitry
Re: [Freedreno] [PATCH] drm/msm/dp: add module parameter for PSR
On Tue, May 23, 2023 at 12:23:04PM -0700, Abhinav Kumar wrote: > On 5/23/2023 8:24 AM, Johan Hovold wrote: > > On Fri, May 12, 2023 at 09:13:04PM +0300, Dmitry Baryshkov wrote: > >> On 28/04/2023 02:28, Abhinav Kumar wrote: > >>> On sc7280 where eDP is the primary display, PSR is causing > >>> IGT breakage even for basic test cases like kms_atomic and > >>> kms_atomic_transition. Most often the issue starts with below > >>> stack so providing that as reference > >>> > >>> Call trace: > >>> ---[ end trace ]--- > >>> [drm-dp] dp_ctrl_push_idle: PUSH_IDLE pattern timedout > >>> > >>> Other basic use-cases still seem to work fine hence add a > >>> a module parameter to allow toggling psr enable/disable till > >>> PSR related issues are hashed out with IGT. > >> > >> For the reference: Bjorn reported that he has issues with VT on a > >> PSR-enabled laptops. This patch fixes the issue for him > > > > Module parameters are almost never warranted, and it is definitely not > > the right way to handle a broken implementation. > > > > I've just sent a revert that unconditionally disables PSR support until > > the implementation has been fixed: > > > > > > https://lore.kernel.org/lkml/20230523151646.28366-1-johan+lin...@kernel.org/ > > I dont completely agree with this. Even the virtual terminal case was > reported to be fixed by one user but not the other. So it was probably > something missed out either in validation or reproduction steps of the > user who reported it to be fixed OR the user who reported it not fixed. > That needs to be investigated now. Yes, there may still be some time left to fix it, but it's pretty damn annoying to find that an issue reported two months ago still is not fixed at 6.4-rc3. (I even waited to make the switch to 6.4 so that I would not have to spend time on this.) I didn't see any mail from Bjorn saying that the series that claimed to fix the VT issue actually did fix the VT issue. There's only the comment above from Dmitry suggesting that disabling this feature is the only way to get a working terminal back. Regressions happen and sometimes there are corner cases that are harder to find, but this is a breakage of a fundamental feature that was reported before the code was even merged into mainline. > We should have ideally gone with the modparam with the feature patches > itself knowing that it gets enabled for all sinks if PSR is supported. Modparams are things of the past should not be used to enable broken features so that some vendor can tick of their internal lists of features that have been "mainlined". You can carry that single patch out-of-tree to enable this if you need it for some particular use case where you don't care about VTs. But hopefully you can just get this sorted quickly. If not, the revert I posted is the way to go rather than adding random module parameters. Johan