Re: [PATCH v3 14/15] drm/msm/dpu: introduce separate wb2_format arrays for rgb and yuv
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar wrote: > > Lets rename the existing wb2_formats array wb2_formats_rgb to indicate > that it has only RGB formats and can be used on any chipset having a WB > block. > > Introduce a new wb2_formats_rgb_yuv array to the catalog to > indicate support for YUV formats to writeback in addition to RGB. > > Chipsets which have support for CDM block will use the newly added > wb2_formats_rgb_yuv array. > > changes in v3: > - change type of wb2_formats_rgb/wb2_formats_rgb_yuv to u32 > to fix checkpatch warnings > > Signed-off-by: Abhinav Kumar > --- > .../msm/disp/dpu1/catalog/dpu_10_0_sm8650.h | 4 +- > .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h| 4 +- > .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h| 4 +- > .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h| 4 +- > .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h| 4 +- > .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 37 ++- > 6 files changed, 46 insertions(+), 11 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 13/15] drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar wrote: > > Reserve CDM blocks for writeback if the format of the output fb > is YUV. At the moment, the reservation is done only for writeback > but can easily be extended by relaxing the checks once other > interfaces are ready to output YUV. > > changes in v3: > - squash CDM disable during encoder cleanup into this change > > changes in v2: > - use needs_cdm from topology struct > - drop fb related checks from atomic_mode_set() > > Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov Minor nit below which I should probably handle while applying the patch. > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 + > 1 file changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 889e9bb42715..989ee8c0e5b4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include > > #include "msm_drv.h" > #include "dpu_kms.h" > @@ -26,6 +27,7 @@ > #include "dpu_hw_dspp.h" > #include "dpu_hw_dsc.h" > #include "dpu_hw_merge3d.h" > +#include "dpu_hw_cdm.h" > #include "dpu_formats.h" > #include "dpu_encoder_phys.h" > #include "dpu_crtc.h" > @@ -582,6 +584,7 @@ static int dpu_encoder_virt_atomic_check( > struct drm_display_mode *adj_mode; > struct msm_display_topology topology; > struct dpu_global_state *global_state; > + struct drm_framebuffer *fb; > struct drm_dsc_config *dsc; > int i = 0; > int ret = 0; > @@ -622,6 +625,22 @@ static int dpu_encoder_virt_atomic_check( > > topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, > crtc_state, dsc); > > + /* > +* Use CDM only for writeback at the moment as other interfaces > cannot handle it. > +* if writeback itself cannot handle cdm for some reason it will fail > in its atomic_check() > +* earlier. What about s/handle/use/ ? > +*/ > + if (dpu_enc->disp_info.intf_type == INTF_WB && > conn_state->writeback_job) { > + fb = conn_state->writeback_job->fb; > + > + if (fb && > DPU_FORMAT_IS_YUV(to_dpu_format(msm_framebuffer_format(fb > + topology.needs_cdm = true; > + if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm) > + crtc_state->mode_changed = true; > + else if (!topology.needs_cdm && dpu_enc->cur_master->hw_cdm) > + crtc_state->mode_changed = true; > + } > + > /* > * Release and Allocate resources on every modeset > * Dont allocate when active is false. > @@ -1062,6 +1081,15 @@ static void dpu_encoder_virt_atomic_mode_set(struct > drm_encoder *drm_enc, > > dpu_enc->dsc_mask = dsc_mask; > > + if (dpu_enc->disp_info.intf_type == INTF_WB && > conn_state->writeback_job) { > + struct dpu_hw_blk *hw_cdm = NULL; > + > + dpu_rm_get_assigned_resources(_kms->rm, global_state, > + drm_enc->base.id, > DPU_HW_BLK_CDM, > + _cdm, 1); > + dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) > : NULL; > + } > + > cstate = to_dpu_crtc_state(crtc_state); > > for (i = 0; i < num_lm; i++) { > @@ -2050,6 +2078,15 @@ void dpu_encoder_helper_phys_cleanup(struct > dpu_encoder_phys *phys_enc) > phys_enc->hw_pp->merge_3d->idx); > } > > + if (phys_enc->hw_cdm) { > + if (phys_enc->hw_cdm->ops.bind_pingpong_blk && > phys_enc->hw_pp) > + > phys_enc->hw_cdm->ops.bind_pingpong_blk(phys_enc->hw_cdm, > + > PINGPONG_NONE); > + if (phys_enc->hw_ctl->ops.update_pending_flush_cdm) > + > phys_enc->hw_ctl->ops.update_pending_flush_cdm(phys_enc->hw_ctl, > + > phys_enc->hw_cdm->idx); > + } > + > if (dpu_enc->dsc) { > dpu_encoder_unprep_dsc(dpu_enc); > dpu_enc->dsc = NULL; > -- > 2.40.1 > -- With best wishes Dmitry
Re: [PATCH v3 11/15] drm/msm/dpu: add an API to setup the CDM block for writeback
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar wrote: > > Add an API dpu_encoder_helper_phys_setup_cdm() which can be used by > the writeback encoder to setup the CDM block. > > Currently, this is defined and used within the writeback's physical > encoder layer however, the function can be modified to be used to setup > the CDM block even for non-writeback interfaces. > > Until those modifications are planned and made, keep it local to > writeback. > > changes in v3: > - call bind_pingpong_blk() directly as disable() is dropped > - add dpu_csc10_rgb2yuv_601l to dpu_hw_util.h and use it > - fix kbot error on the function doc > - document that dpu_encoder_helper_phys_setup_cdm() doesn't handle > DPU_CHROMA_H1V2 > > changes in v2: > - add the RGB2YUV CSC matrix to dpu util as needed by CDM > - use dpu_hw_get_csc_cfg() to get and program CSC > - drop usage of setup_csc_data() and setup_cdwn() cdm ops > as they both have been merged into enable() > - drop reduntant hw_cdm and hw_pp checks > > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202312102149.qmbcdsg2-...@intel.com/ > Signed-off-by: Abhinav Kumar > --- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 ++ > .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 93 ++- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 14 +++ > 3 files changed, 112 insertions(+), 1 deletion(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v2 05/16] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
On Mon, 11 Dec 2023 at 23:48, Abhinav Kumar wrote: > > > > On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote: > > On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar > > wrote: > >> > >> > >> > >> On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: > >>> On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar > >>> wrote: > > > > On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar > > wrote: > >> > >> Add CDM blocks to the sc7280 dpu_hw_catalog to support > >> YUV format output from writeback block. > >> > >> changes in v2: > >>- remove explicit zero assignment for features > >>- move sc7280_cdm to dpu_hw_catalog from the sc7280 > >> catalog file as its definition can be re-used > >> > >> Signed-off-by: Abhinav Kumar > >> --- > >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 > >> + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 + > >> 4 files changed, 29 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> index 209675de6742..19c2b7454796 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > >>.mdss_ver = _mdss_ver, > >>.caps = _dpu_caps, > >>.mdp = _mdp, > >> + .cdm = _cdm, > >>.ctl_count = ARRAY_SIZE(sc7280_ctl), > >>.ctl = sc7280_ctl, > >>.sspp_count = ARRAY_SIZE(sc7280_sspp), > >> 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 d52aae54bbd5..1be3156cde05 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = > >> { > >>.ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > >> }; > >> > >> +/* > >> + * CDM sub block config > > > > Nit: it is not a subblock config. > > > > Ack. > > >> + */ > >> +static const struct dpu_cdm_cfg sc7280_cdm = { > > > > I know that I have r-b'ed this patch. But then one thing occurred to > > me. If this definition is common to all (or almost all) platforms, can > > we just call it dpu_cdm or dpu_common_cdm? > > > >> + .name = "cdm_0", > >> + .id = CDM_0, > >> + .len = 0x228, > >> + .base = 0x79200, > >> +}; > > hmmm almost common but not entirely ... msm8998's CDM has a shorter > len of 0x224 :( > >>> > >>> Then sdm845_cdm? > >>> > >> > >> That also has a shorter cdm length. > > > > Could you please clarify. According to the downstream DT files all CDM > > blocks up to (but not including) sm8550 had length 0x224. SM8550 and > > SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers > > after 0x204. > >> > > We always list 0x4 more than the last offset. Yes, so this makes it correct for several latest DT files, which have qcom,sde-cdm-size = <0x220>. However all the previous DT files (from msm8998 to sm8450) had qcom,sde-cdm-size = <0x224>; > > In chipsets sdm845 and msm8998, I only see the last offset of CDM as > 0x220 but in sm8250 and sc7280, the last offset is 0x224. Hence the > total length is more in sc7280/sm8250 as compared to sdm845/msm8998. > > I didnt follow that you do not see any registers after 0x204. > > The CDM_MUX is the last offset which has an offset 0x224 from the base > address. So thats the last offset. Ack > > The newer chipsets have CDM_MUX and the older ones did not. Hence the > difference in length. > > >> BTW, sdm845 is not in this series. It will be part of RFT as we discussed. > >> > > >> + > >> /* > >> * VBIF sub blocks config > >> */ > >> 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 e3c0d007481b..ba82ef4560a6 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > >>u32
Re: [PATCH v3 04/15] drm/msm/dpu: move csc matrices to dpu_hw_util
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar wrote: > > Since the type and usage of CSC matrices is spanning across DPU > lets introduce a helper to the dpu_hw_util to return the CSC > corresponding to the request type. This will help to add more > supported CSC types such as the RGB to YUV one which is used in > the case of CDM. > > changes in v3: > - drop the extra wrapper and export the matrices directly > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 30 > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 + > 2 files changed, 31 insertions(+), 30 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v3 01/15] drm/msm/dpu: add formats check for writeback encoder
On Tue, 12 Dec 2023 at 02:23, Abhinav Kumar wrote: > > In preparation for adding more formats to dpu writeback add > format validation to it to fail any unsupported formats. > > changes in v3: > - rebase on top of msm-next > - replace drm_atomic_helper_check_wb_encoder_state() with > drm_atomic_helper_check_wb_connector_state() due to the > rebase > > changes in v2: > - correct some grammar in the commit text > > Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for > writeback") > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > index bb94909caa25..425415d45ec1 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > @@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check( > { > struct drm_framebuffer *fb; > const struct drm_display_mode *mode = _state->mode; > + int ret; > > DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n", > phys_enc->hw_wb->idx, mode->name, mode->hdisplay, > mode->vdisplay); > @@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check( > return -EINVAL; > } > > + ret = > drm_atomic_helper_check_wb_connector_state(conn_state->connector, > conn_state->state); > + if (ret < 0) { > + DPU_ERROR("invalid pixel format %p4cc\n", > >format->format); > + return ret; > + } There is no guarantee that there will be no other checks added to this helper. So, I think this message is incorrect. If you wish, you can promote the level of the message in the helper itself. On the other hand, we rarely print such messages by default. Most of the checks use drm_dbg. > + > return 0; > } > > -- > 2.40.1 > -- With best wishes Dmitry
Re: [PATCH v3] drm/msm/dpu: improve DSC allocation
On Tue, 12 Dec 2023 at 02:03, Kuogee Hsieh wrote: > > > On 12/11/2023 1:30 PM, Dmitry Baryshkov wrote: > > On Mon, 11 Dec 2023 at 20:38, Kuogee Hsieh wrote: > >> A DCE (Display Compression Engine) contains two DSC hard slice > >> encoders. Each DCE start with even DSC encoder index followed by > > "starts". But it will not be correct. The DCE doesn't start with the > > DSC encoder. DCE consists of two DSC encoders, one has an odd index > > and another one has an even index. > > > >> an odd DSC encoder index. Each encoder can work independently. > >> But Only two DSC encoders from same DCE can be paired to work > > only > > > >> together to support merge mode. In addition, the DSC with even > > There are different merge modes. Here you are talking about the DSC merge > > mode. > > > >> index have to mapping to even pingpong index and DSC with odd > > PINGPONG (end everywhere else). > > > > have to be mapped, should be used, etc. > > > >> index have to mapping to odd pingpong index at its data path. > >> This patch improve DSC allocation mechanism with consideration > > improves > > > >> of above factors. > > of these factors. > > > >> Changes in V3: > >> -- add dpu_rm_pingpong_dsc_check() > >> -- for pair allocation use i += 2 at for loop > >> > >> Changes in V2: > >> -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and > >> _dpu_rm_reserve_dsc_pair() > >> > >> Fixes: f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in RM") > > This tag is incorrect. The patch should be split into two pieces. One > > which fixes DSC allocation for DSC 1.1 encoders, where there were no > > DCE blocks, another one which adds proper handling for DCE. > > Unless the paired allocation requirement also applies to pre-DCE DSC > > encoders. But in that case the commit message doesn't make any sense. > > > > I checked 4.x Qualcomm kernels. None of them contained any of these > > restrictions for DSC blocks. Only the displaypack targeting 4.19 > > kernel got these changes. But it predates DCE pairs support. > > as I said earlier the rule of odd/even pp-index map to odd/even > dsc-index is there since dsc v1.1. > > I think current code (including down stream code) works by luck to not > encounter a configuration with two independence paths, one with single > dsc and the other one use two dsc to support dsc merge mode. > > this patch is the fix to enforce this rule for both dsc v1.1 and v1.2 > and I will rework commit message yo have better description. Good. Does this apply to paired allocation too? I think so, as the techpack first got the paired allocation and only afterwards it has got the DSC/PP idx check. Regarding the patch itself. May I suggest an alternative approach, which should work better, I think. At least it will not require 'deleting' the PP indices. First you preprocess the pp_to_enc_id array and list all PP indices selected for this encoder. Then you work with this array, matching PP and DSC blocks. -- With best wishes Dmitry
Re: [RFT PATCH v2 0/4] drm/msm/dpu: enable writeback on the other platforms
On Tue, 12 Dec 2023 at 02:30, Abhinav Kumar wrote: > > > > On 12/2/2023 4:31 PM, Dmitry Baryshkov wrote: > > I was not able to test it on my own, this is a call for testing for the > > owners of these platforms. The git version of modetest now fully > > supports writeback. > > > > Use libdrm >= 2.4.117, run modetest -ac to determine the writeback > > connector, cat /sys/kernel/debug/dri/0/state to determine > > spare CRTC and plane, then run something like: > > > > modetest -M msm -a -s 36@85:1024x768 -o test.d -P 79@85:1024x768 > > > > where 36 is the Writeback connector id, 85 is CRTC and 79 is the plane. > > > > Then press Enter and check the test.d file for the raw image dump. > > > > Changes since v1: > > - Fixed the DPU_CLK_CTRL_WB2 definition > > > > I think this series needs to be re-based as WB_SDM845_MASK is no longer > present in msm-next and 3/4 patches in this series use that. Quite the contrary: the WB_SDM845_MASK was added in https://patchwork.freedesktop.org/patch/570189/?series=127245=1, which is now merged to msm-next-lumag > > > Dmitry Baryshkov (4): > >drm/msm/dpu: enable writeback on SM8150 > >drm/msm/dpu: enable writeback on SC8108X > >drm/msm/dpu: enable writeback on SM6125 > >drm/msm/dpu: enable writeback on SM6350 > > > > .../drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 18 ++ > > .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 18 ++ > > .../drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 18 ++ > > .../drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h | 18 ++ > > 4 files changed, 72 insertions(+) > > -- With best wishes Dmitry
Re: [RFT PATCH v2 0/4] drm/msm/dpu: enable writeback on the other platforms
On 12/2/2023 4:31 PM, Dmitry Baryshkov wrote: I was not able to test it on my own, this is a call for testing for the owners of these platforms. The git version of modetest now fully supports writeback. Use libdrm >= 2.4.117, run modetest -ac to determine the writeback connector, cat /sys/kernel/debug/dri/0/state to determine spare CRTC and plane, then run something like: modetest -M msm -a -s 36@85:1024x768 -o test.d -P 79@85:1024x768 where 36 is the Writeback connector id, 85 is CRTC and 79 is the plane. Then press Enter and check the test.d file for the raw image dump. Changes since v1: - Fixed the DPU_CLK_CTRL_WB2 definition I think this series needs to be re-based as WB_SDM845_MASK is no longer present in msm-next and 3/4 patches in this series use that. Dmitry Baryshkov (4): drm/msm/dpu: enable writeback on SM8150 drm/msm/dpu: enable writeback on SC8108X drm/msm/dpu: enable writeback on SM6125 drm/msm/dpu: enable writeback on SM6350 .../drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 18 ++ .../msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 18 ++ .../drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 18 ++ .../drm/msm/disp/dpu1/catalog/dpu_6_4_sm6350.h | 18 ++ 4 files changed, 72 insertions(+)
[PATCH v3 10/15] drm/msm/dpu: add CDM related logic to dpu_hw_ctl layer
CDM block will need its own logic to program the flush and active bits in the dpu_hw_ctl layer. Make necessary changes in dpu_hw_ctl to support CDM programming. changes in v3: - drop unused cdm_active as reported by kbot - retained the R-b as its a trivial change changes in v2: - remove unused empty line - pass in cdm_num to update_pending_flush_cdm() Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202312102047.s0i69pcs-...@intel.com/ Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 33 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 12 2 files changed, 45 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 e7b680a151d6..e76565c3e6a4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -32,11 +32,13 @@ #define CTL_DSC_ACTIVE0x0E8 #define CTL_WB_ACTIVE 0x0EC #define CTL_INTF_ACTIVE 0x0F4 +#define CTL_CDM_ACTIVE0x0F8 #define CTL_FETCH_PIPE_ACTIVE 0x0FC #define CTL_MERGE_3D_FLUSH0x100 #define CTL_DSC_FLUSH0x104 #define CTL_WB_FLUSH 0x108 #define CTL_INTF_FLUSH0x110 +#define CTL_CDM_FLUSH0x114 #define CTL_INTF_MASTER 0x134 #define CTL_DSPP_n_FLUSH(n) ((0x13C) + ((n) * 4)) @@ -46,6 +48,7 @@ #define DPU_REG_RESET_TIMEOUT_US2000 #define MERGE_3D_IDX 23 #define DSC_IDX22 +#define CDM_IDX 26 #define INTF_IDX 31 #define WB_IDX 16 #define DSPP_IDX 29 /* From DPU hw rev 7.x.x */ @@ -107,6 +110,7 @@ static inline void dpu_hw_ctl_clear_pending_flush(struct dpu_hw_ctl *ctx) ctx->pending_wb_flush_mask = 0; ctx->pending_merge_3d_flush_mask = 0; ctx->pending_dsc_flush_mask = 0; + ctx->pending_cdm_flush_mask = 0; memset(ctx->pending_dspp_flush_mask, 0, sizeof(ctx->pending_dspp_flush_mask)); @@ -151,6 +155,10 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx) DPU_REG_WRITE(>hw, CTL_DSC_FLUSH, ctx->pending_dsc_flush_mask); + if (ctx->pending_flush_mask & BIT(CDM_IDX)) + DPU_REG_WRITE(>hw, CTL_CDM_FLUSH, + ctx->pending_cdm_flush_mask); + DPU_REG_WRITE(>hw, CTL_FLUSH, ctx->pending_flush_mask); } @@ -282,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_wb(struct dpu_hw_ctl *ctx, } } +static void dpu_hw_ctl_update_pending_flush_cdm(struct dpu_hw_ctl *ctx, enum dpu_cdm cdm_num) +{ + /* update pending flush only if CDM_0 is flushed */ + if (cdm_num == CDM_0) + ctx->pending_flush_mask |= BIT(CDM_IDX); +} + static void dpu_hw_ctl_update_pending_flush_wb_v1(struct dpu_hw_ctl *ctx, enum dpu_wb wb) { @@ -310,6 +325,12 @@ static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx, ctx->pending_flush_mask |= BIT(DSC_IDX); } +static void dpu_hw_ctl_update_pending_flush_cdm_v1(struct dpu_hw_ctl *ctx, enum dpu_cdm cdm_num) +{ + ctx->pending_cdm_flush_mask |= BIT(cdm_num - CDM_0); + ctx->pending_flush_mask |= BIT(CDM_IDX); +} + static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx, enum dpu_dspp dspp, u32 dspp_sub_blk) { @@ -543,6 +564,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx, if (cfg->dsc) DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc); + + if (cfg->cdm) + DPU_REG_WRITE(c, CTL_CDM_ACTIVE, cfg->cdm); } static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx, @@ -586,6 +610,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx, u32 wb_active = 0; u32 merge3d_active = 0; u32 dsc_active; + u32 cdm_active; /* * This API resets each portion of the CTL path namely, @@ -621,6 +646,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx, dsc_active &= ~cfg->dsc; DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active); } + + if (cfg->cdm) { + cdm_active = DPU_REG_READ(c, CTL_CDM_ACTIVE); + cdm_active &= ~cfg->cdm; + DPU_REG_WRITE(c, CTL_CDM_ACTIVE, cdm_active); + } } static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx, @@ -654,12 +685,14 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops, ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1; ops->update_pending_flush_dsc = dpu_hw_ctl_update_pending_flush_dsc_v1; + ops->update_pending_flush_cdm =
[PATCH v3 13/15] drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output
Reserve CDM blocks for writeback if the format of the output fb is YUV. At the moment, the reservation is done only for writeback but can easily be extended by relaxing the checks once other interfaces are ready to output YUV. changes in v3: - squash CDM disable during encoder cleanup into this change changes in v2: - use needs_cdm from topology struct - drop fb related checks from atomic_mode_set() Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 + 1 file changed, 37 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 889e9bb42715..989ee8c0e5b4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -16,6 +16,7 @@ #include #include #include +#include #include "msm_drv.h" #include "dpu_kms.h" @@ -26,6 +27,7 @@ #include "dpu_hw_dspp.h" #include "dpu_hw_dsc.h" #include "dpu_hw_merge3d.h" +#include "dpu_hw_cdm.h" #include "dpu_formats.h" #include "dpu_encoder_phys.h" #include "dpu_crtc.h" @@ -582,6 +584,7 @@ static int dpu_encoder_virt_atomic_check( struct drm_display_mode *adj_mode; struct msm_display_topology topology; struct dpu_global_state *global_state; + struct drm_framebuffer *fb; struct drm_dsc_config *dsc; int i = 0; int ret = 0; @@ -622,6 +625,22 @@ static int dpu_encoder_virt_atomic_check( topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, dsc); + /* +* Use CDM only for writeback at the moment as other interfaces cannot handle it. +* if writeback itself cannot handle cdm for some reason it will fail in its atomic_check() +* earlier. +*/ + if (dpu_enc->disp_info.intf_type == INTF_WB && conn_state->writeback_job) { + fb = conn_state->writeback_job->fb; + + if (fb && DPU_FORMAT_IS_YUV(to_dpu_format(msm_framebuffer_format(fb + topology.needs_cdm = true; + if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm) + crtc_state->mode_changed = true; + else if (!topology.needs_cdm && dpu_enc->cur_master->hw_cdm) + crtc_state->mode_changed = true; + } + /* * Release and Allocate resources on every modeset * Dont allocate when active is false. @@ -1062,6 +1081,15 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc, dpu_enc->dsc_mask = dsc_mask; + if (dpu_enc->disp_info.intf_type == INTF_WB && conn_state->writeback_job) { + struct dpu_hw_blk *hw_cdm = NULL; + + dpu_rm_get_assigned_resources(_kms->rm, global_state, + drm_enc->base.id, DPU_HW_BLK_CDM, + _cdm, 1); + dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL; + } + cstate = to_dpu_crtc_state(crtc_state); for (i = 0; i < num_lm; i++) { @@ -2050,6 +2078,15 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc) phys_enc->hw_pp->merge_3d->idx); } + if (phys_enc->hw_cdm) { + if (phys_enc->hw_cdm->ops.bind_pingpong_blk && phys_enc->hw_pp) + phys_enc->hw_cdm->ops.bind_pingpong_blk(phys_enc->hw_cdm, + PINGPONG_NONE); + if (phys_enc->hw_ctl->ops.update_pending_flush_cdm) + phys_enc->hw_ctl->ops.update_pending_flush_cdm(phys_enc->hw_ctl, + phys_enc->hw_cdm->idx); + } + if (dpu_enc->dsc) { dpu_encoder_unprep_dsc(dpu_enc); dpu_enc->dsc = NULL; -- 2.40.1
[PATCH v3 07/15] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block
CDM block comes with its own set of registers and operations which can be done. In-line with other hardware blocks, this change adds the dpu_hw_cdm abstraction for the CDM block. changes in v3: - fix commit text from sub-blk to blk for CDM - fix kbot issue for missing static for dpu_hw_cdm_enable() - fix kbot issue for incorrect documentation style - add more documentation for enums and struct in dpu_hw_cdm.h - drop "enable" parameter from bind_pingpong_blk() as we can just use PINGPONG_NONE for disable cases - drop unnecessary bit operation for zero value of cdm_cfg changes in v2: - replace bit magic with relevant defines - use drmm_kzalloc instead of kzalloc/free - some formatting fixes - inline _setup_cdm_ops() - protect bind_pingpong_blk with core_rev check - drop setup_csc_data() and setup_cdwn() ops as they are merged into enable() Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202312101815.b3zh7pfy-...@intel.com/ Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/Makefile| 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 263 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 130 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + 4 files changed, 395 insertions(+) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 49671364fdcf..b1173128b5b9 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_encoder_phys_wb.o \ disp/dpu1/dpu_formats.o \ disp/dpu1/dpu_hw_catalog.o \ + disp/dpu1/dpu_hw_cdm.o \ disp/dpu1/dpu_hw_ctl.o \ disp/dpu1/dpu_hw_dsc.o \ disp/dpu1/dpu_hw_dsc_1_2.o \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c new file mode 100644 index ..4976f8a05ce7 --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c @@ -0,0 +1,263 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2023, The Linux Foundation. All rights reserved. + */ + +#include + +#include "dpu_hw_mdss.h" +#include "dpu_hw_util.h" +#include "dpu_hw_catalog.h" +#include "dpu_hw_cdm.h" +#include "dpu_kms.h" + +#define CDM_CSC_10_OPMODE 0x000 +#define CDM_CSC_10_BASE0x004 + +#define CDM_CDWN2_OP_MODE 0x100 +#define CDM_CDWN2_CLAMP_OUT0x104 +#define CDM_CDWN2_PARAMS_3D_0 0x108 +#define CDM_CDWN2_PARAMS_3D_1 0x10C +#define CDM_CDWN2_COEFF_COSITE_H_0 0x110 +#define CDM_CDWN2_COEFF_COSITE_H_1 0x114 +#define CDM_CDWN2_COEFF_COSITE_H_2 0x118 +#define CDM_CDWN2_COEFF_OFFSITE_H_00x11C +#define CDM_CDWN2_COEFF_OFFSITE_H_10x120 +#define CDM_CDWN2_COEFF_OFFSITE_H_20x124 +#define CDM_CDWN2_COEFF_COSITE_V 0x128 +#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C +#define CDM_CDWN2_OUT_SIZE 0x130 + +#define CDM_HDMI_PACK_OP_MODE 0x200 +#define CDM_CSC_10_MATRIX_COEFF_0 0x004 + +#define CDM_MUX0x224 + +/* CDM CDWN2 sub-block bit definitions */ +#define CDM_CDWN2_OP_MODE_EN BIT(0) +#define CDM_CDWN2_OP_MODE_ENABLE_HBIT(1) +#define CDM_CDWN2_OP_MODE_ENABLE_VBIT(2) +#define CDM_CDWN2_OP_MODE_METHOD_H_AVGBIT(3) +#define CDM_CDWN2_OP_MODE_METHOD_H_COSITE BIT(4) +#define CDM_CDWN2_OP_MODE_METHOD_V_AVGBIT(5) +#define CDM_CDWN2_OP_MODE_METHOD_V_COSITE BIT(6) +#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7) +#define CDM_CDWN2_OP_MODE_METHOD_H_OFFSITEGENMASK(4, 3) +#define CDM_CDWN2_OP_MODE_METHOD_V_OFFSITEGENMASK(6, 5) +#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5) +#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3) + +/* CDM CSC10 sub-block bit definitions */ +#define CDM_CSC10_OP_MODE_EN BIT(0) +#define CDM_CSC10_OP_MODE_SRC_FMT_YUV BIT(1) +#define CDM_CSC10_OP_MODE_DST_FMT_YUV BIT(2) + +/* CDM HDMI pack sub-block bit definitions */ +#define CDM_HDMI_PACK_OP_MODE_EN BIT(0) + +/* + * Horizontal coefficients for cosite chroma downscale + * s13 representation of coefficients + */ +static u32 cosite_h_coeff[] = {0x0016, 0x01cc, 0x019e}; + +/* + * Horizontal coefficients for offsite chroma downscale + */ +static u32 offsite_h_coeff[] = {0x000b0005, 0x01db01eb, 0x00e40046}; + +/* + * Vertical coefficients for cosite chroma downscale + */ +static u32 cosite_v_coeff[] = {0x00080004}; +/* + * Vertical coefficients for offsite chroma downscale + */ +static u32 offsite_v_coeff[] = {0x00060002}; + +static int
[PATCH v3 12/15] drm/msm/dpu: plug-in the cdm related bits to writeback setup
To setup and enable CDM block for the writeback pipeline, lets add the pieces together to set the active bits and the flush bits for the CDM block. changes in v2: - passed the cdm idx to update_pending_flush_cdm() (have retained the R-b as its a minor change) Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index 85cb8596737d..0a28198886d5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -214,6 +214,7 @@ static void dpu_encoder_phys_wb_setup_ctl(struct dpu_encoder_phys *phys_enc) { struct dpu_hw_wb *hw_wb; struct dpu_hw_ctl *ctl; + struct dpu_hw_cdm *hw_cdm; if (!phys_enc) { DPU_ERROR("invalid encoder\n"); @@ -222,6 +223,7 @@ static void dpu_encoder_phys_wb_setup_ctl(struct dpu_encoder_phys *phys_enc) hw_wb = phys_enc->hw_wb; ctl = phys_enc->hw_ctl; + hw_cdm = phys_enc->hw_cdm; if (test_bit(DPU_CTL_ACTIVE_CFG, >caps->features) && (phys_enc->hw_ctl && @@ -238,6 +240,9 @@ static void dpu_encoder_phys_wb_setup_ctl(struct dpu_encoder_phys *phys_enc) if (mode_3d && hw_pp && hw_pp->merge_3d) intf_cfg.merge_3d = hw_pp->merge_3d->idx; + if (hw_cdm) + intf_cfg.cdm = hw_cdm->idx; + if (phys_enc->hw_pp->merge_3d && phys_enc->hw_pp->merge_3d->ops.setup_3d_mode) phys_enc->hw_pp->merge_3d->ops.setup_3d_mode(phys_enc->hw_pp->merge_3d, mode_3d); @@ -418,6 +423,7 @@ static void _dpu_encoder_phys_wb_update_flush(struct dpu_encoder_phys *phys_enc) struct dpu_hw_wb *hw_wb; struct dpu_hw_ctl *hw_ctl; struct dpu_hw_pingpong *hw_pp; + struct dpu_hw_cdm *hw_cdm; u32 pending_flush = 0; if (!phys_enc) @@ -426,6 +432,7 @@ static void _dpu_encoder_phys_wb_update_flush(struct dpu_encoder_phys *phys_enc) hw_wb = phys_enc->hw_wb; hw_pp = phys_enc->hw_pp; hw_ctl = phys_enc->hw_ctl; + hw_cdm = phys_enc->hw_cdm; DPU_DEBUG("[wb:%d]\n", hw_wb->idx - WB_0); @@ -441,6 +448,9 @@ static void _dpu_encoder_phys_wb_update_flush(struct dpu_encoder_phys *phys_enc) hw_ctl->ops.update_pending_flush_merge_3d(hw_ctl, hw_pp->merge_3d->idx); + if (hw_cdm && hw_ctl->ops.update_pending_flush_cdm) + hw_ctl->ops.update_pending_flush_cdm(hw_ctl, hw_cdm->idx); + if (hw_ctl->ops.get_pending_flush) pending_flush = hw_ctl->ops.get_pending_flush(hw_ctl); -- 2.40.1
[PATCH v3 15/15] drm/msm/dpu: add cdm blocks to dpu snapshot
Now that CDM block support has been added to DPU lets also add its entry to the DPU snapshot to help debugging. Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index dc24fe4bb3b0..59647ad19906 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -947,6 +947,10 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k } } + if (cat->cdm) + msm_disp_snapshot_add_block(disp_state, cat->cdm->len, + dpu_kms->mmio + cat->cdm->base, cat->cdm->name); + pm_runtime_put_sync(_kms->pdev->dev); } -- 2.40.1
[PATCH v3 14/15] drm/msm/dpu: introduce separate wb2_format arrays for rgb and yuv
Lets rename the existing wb2_formats array wb2_formats_rgb to indicate that it has only RGB formats and can be used on any chipset having a WB block. Introduce a new wb2_formats_rgb_yuv array to the catalog to indicate support for YUV formats to writeback in addition to RGB. Chipsets which have support for CDM block will use the newly added wb2_formats_rgb_yuv array. changes in v3: - change type of wb2_formats_rgb/wb2_formats_rgb_yuv to u32 to fix checkpatch warnings Signed-off-by: Abhinav Kumar --- .../msm/disp/dpu1/catalog/dpu_10_0_sm8650.h | 4 +- .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h| 4 +- .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h| 4 +- .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h| 4 +- .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h| 4 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 37 ++- 6 files changed, 46 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h index 04d2a73dd942..eb5dfff2ec4f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h @@ -341,8 +341,8 @@ static const struct dpu_wb_cfg sm8650_wb[] = { .name = "wb_2", .id = WB_2, .base = 0x65000, .len = 0x2c8, .features = WB_SM8250_MASK, - .format_list = wb2_formats, - .num_formats = ARRAY_SIZE(wb2_formats), + .format_list = wb2_formats_rgb, + .num_formats = ARRAY_SIZE(wb2_formats_rgb), .xin_id = 6, .vbif_idx = VBIF_RT, .maxlinewidth = 4096, diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h index 58b0f50518c8..a57d50b1f028 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h @@ -336,8 +336,8 @@ static const struct dpu_wb_cfg sm8250_wb[] = { .name = "wb_2", .id = WB_2, .base = 0x65000, .len = 0x2c8, .features = WB_SM8250_MASK, - .format_list = wb2_formats, - .num_formats = ARRAY_SIZE(wb2_formats), + .format_list = wb2_formats_rgb_yuv, + .num_formats = ARRAY_SIZE(wb2_formats_rgb_yuv), .clk_ctrl = DPU_CLK_CTRL_WB2, .xin_id = 6, .vbif_idx = VBIF_RT, diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h index bcfedfc8251a..7382ebb6e5b2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h @@ -157,8 +157,8 @@ static const struct dpu_wb_cfg sc7180_wb[] = { .name = "wb_2", .id = WB_2, .base = 0x65000, .len = 0x2c8, .features = WB_SM8250_MASK, - .format_list = wb2_formats, - .num_formats = ARRAY_SIZE(wb2_formats), + .format_list = wb2_formats_rgb, + .num_formats = ARRAY_SIZE(wb2_formats_rgb), .clk_ctrl = DPU_CLK_CTRL_WB2, .xin_id = 6, .vbif_idx = VBIF_RT, diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 19c2b7454796..2f153e0b5c6a 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -169,8 +169,8 @@ static const struct dpu_wb_cfg sc7280_wb[] = { .name = "wb_2", .id = WB_2, .base = 0x65000, .len = 0x2c8, .features = WB_SM8250_MASK, - .format_list = wb2_formats, - .num_formats = ARRAY_SIZE(wb2_formats), + .format_list = wb2_formats_rgb_yuv, + .num_formats = ARRAY_SIZE(wb2_formats_rgb_yuv), .clk_ctrl = DPU_CLK_CTRL_WB2, .xin_id = 6, .vbif_idx = VBIF_RT, diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h index bf56265967c0..ad48defa154f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h @@ -315,8 +315,8 @@ static const struct dpu_wb_cfg sm8550_wb[] = { .name = "wb_2", .id = WB_2, .base = 0x65000, .len = 0x2c8, .features = WB_SM8250_MASK, - .format_list = wb2_formats, - .num_formats = ARRAY_SIZE(wb2_formats), + .format_list = wb2_formats_rgb, + .num_formats = ARRAY_SIZE(wb2_formats_rgb), .xin_id = 6, .vbif_idx = VBIF_RT, .maxlinewidth = 4096, diff --git
[PATCH v3 06/15] drm/msm/dpu: add cdm blocks to sm8250 dpu_hw_catalog
Add CDM blocks to the sm8250 dpu_hw_catalog to support YUV format output from writeback block. changes in v2: - re-use the cdm definition from sc7280 Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h index 2359c16e9206..58b0f50518c8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h @@ -384,6 +384,7 @@ const struct dpu_mdss_cfg dpu_sm8250_cfg = { .mdss_ver = _mdss_ver, .caps = _dpu_caps, .mdp = _mdp, + .cdm = _cdm, .ctl_count = ARRAY_SIZE(sm8250_ctl), .ctl = sm8250_ctl, .sspp_count = ARRAY_SIZE(sm8250_sspp), -- 2.40.1
[PATCH v3 11/15] drm/msm/dpu: add an API to setup the CDM block for writeback
Add an API dpu_encoder_helper_phys_setup_cdm() which can be used by the writeback encoder to setup the CDM block. Currently, this is defined and used within the writeback's physical encoder layer however, the function can be modified to be used to setup the CDM block even for non-writeback interfaces. Until those modifications are planned and made, keep it local to writeback. changes in v3: - call bind_pingpong_blk() directly as disable() is dropped - add dpu_csc10_rgb2yuv_601l to dpu_hw_util.h and use it - fix kbot error on the function doc - document that dpu_encoder_helper_phys_setup_cdm() doesn't handle DPU_CHROMA_H1V2 changes in v2: - add the RGB2YUV CSC matrix to dpu util as needed by CDM - use dpu_hw_get_csc_cfg() to get and program CSC - drop usage of setup_csc_data() and setup_cdwn() cdm ops as they both have been merged into enable() - drop reduntant hw_cdm and hw_pp checks Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202312102149.qmbcdsg2-...@intel.com/ Signed-off-by: Abhinav Kumar --- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 ++ .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 93 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 14 +++ 3 files changed, 112 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index e2934a6702d1..bdb89cbbcfb8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -14,8 +14,10 @@ #include "dpu_hw_intf.h" #include "dpu_hw_wb.h" #include "dpu_hw_pingpong.h" +#include "dpu_hw_cdm.h" #include "dpu_hw_ctl.h" #include "dpu_hw_top.h" +#include "dpu_hw_util.h" #include "dpu_encoder.h" #include "dpu_crtc.h" @@ -150,6 +152,7 @@ enum dpu_intr_idx { * @hw_pp: Hardware interface to the ping pong registers * @hw_intf: Hardware interface to the intf registers * @hw_wb: Hardware interface to the wb registers + * @hw_cdm:Hardware interface to the CDM registers * @dpu_kms: Pointer to the dpu_kms top level * @cached_mode: DRM mode cached at mode_set time, acted on in enable * @enabled: Whether the encoder has enabled and running a mode @@ -178,6 +181,7 @@ struct dpu_encoder_phys { struct dpu_hw_pingpong *hw_pp; struct dpu_hw_intf *hw_intf; struct dpu_hw_wb *hw_wb; + struct dpu_hw_cdm *hw_cdm; struct dpu_kms *dpu_kms; struct drm_display_mode cached_mode; enum dpu_enc_split_role split_role; @@ -207,6 +211,7 @@ static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys) * @wbirq_refcount: Reference count of writeback interrupt * @wb_done_timeout_cnt: number of wb done irq timeout errors * @wb_cfg: writeback block config to store fb related details + * @cdm_cfg: cdm block config needed to store writeback block's CDM configuration * @wb_conn: backpointer to writeback connector * @wb_job: backpointer to current writeback job * @dest: dpu buffer layout for current writeback output buffer @@ -216,6 +221,7 @@ struct dpu_encoder_phys_wb { atomic_t wbirq_refcount; int wb_done_timeout_cnt; struct dpu_hw_wb_cfg wb_cfg; + struct dpu_hw_cdm_cfg cdm_cfg; struct drm_writeback_connector *wb_conn; struct drm_writeback_job *wb_job; struct dpu_hw_fmt_layout dest; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index 8f05f2a1fc24..85cb8596737d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -259,6 +259,96 @@ static void dpu_encoder_phys_wb_setup_ctl(struct dpu_encoder_phys *phys_enc) } } +/** + * dpu_encoder_helper_phys_setup_cdm - setup chroma down sampling block + * This API does not handle DPU_CHROMA_H1V2. + * @phys_enc:Pointer to physical encoder + */ +static void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc) +{ + struct dpu_hw_cdm *hw_cdm; + struct dpu_hw_cdm_cfg *cdm_cfg; + struct dpu_hw_pingpong *hw_pp; + struct dpu_encoder_phys_wb *wb_enc; + const struct msm_format *format; + const struct dpu_format *dpu_fmt; + struct drm_writeback_job *wb_job; + int ret; + + if (!phys_enc) + return; + + wb_enc = to_dpu_encoder_phys_wb(phys_enc); + cdm_cfg = _enc->cdm_cfg; + hw_pp = phys_enc->hw_pp; + hw_cdm = phys_enc->hw_cdm; + wb_job = wb_enc->wb_job; + + format = msm_framebuffer_format(wb_enc->wb_job->fb); + dpu_fmt = dpu_get_dpu_format_ext(format->pixel_format, wb_job->fb->modifier); + + if (!hw_cdm) + return; + +
[PATCH v3 09/15] drm/msm/dpu: add support to allocate CDM from RM
Even though there is usually only one CDM block, it can be used by either HDMI, DisplayPort OR Writeback interfaces. Hence its allocation needs to be tracked properly by the resource manager to ensure appropriate availability of the block. changes in v2: - move needs_cdm to topology struct Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 38 +++-- drivers/gpu/drm/msm/msm_drv.h | 2 ++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h index 9db4cf61bd29..5df545904057 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h @@ -98,6 +98,7 @@ enum dpu_hw_blk_type { DPU_HW_BLK_DSPP, DPU_HW_BLK_MERGE_3D, DPU_HW_BLK_DSC, + DPU_HW_BLK_CDM, DPU_HW_BLK_MAX, }; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index df6271017b80..a0cd36e45a01 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -135,6 +135,7 @@ struct dpu_global_state { uint32_t ctl_to_enc_id[CTL_MAX - CTL_0]; uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0]; uint32_t dsc_to_enc_id[DSC_MAX - DSC_0]; + uint32_t cdm_to_enc_id; }; struct dpu_global_state diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 7ed476b96304..b58a9c2ae326 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -435,6 +435,26 @@ static int _dpu_rm_reserve_dsc(struct dpu_rm *rm, return 0; } +static int _dpu_rm_reserve_cdm(struct dpu_rm *rm, + struct dpu_global_state *global_state, + struct drm_encoder *enc) +{ + /* try allocating only one CDM block */ + if (!rm->cdm_blk) { + DPU_ERROR("CDM block does not exist\n"); + return -EIO; + } + + if (global_state->cdm_to_enc_id) { + DPU_ERROR("CDM_0 is already allocated\n"); + return -EIO; + } + + global_state->cdm_to_enc_id = enc->base.id; + + return 0; +} + static int _dpu_rm_make_reservation( struct dpu_rm *rm, struct dpu_global_state *global_state, @@ -460,6 +480,14 @@ static int _dpu_rm_make_reservation( if (ret) return ret; + if (reqs->topology.needs_cdm) { + ret = _dpu_rm_reserve_cdm(rm, global_state, enc); + if (ret) { + DPU_ERROR("unable to find CDM blk\n"); + return ret; + } + } + return ret; } @@ -470,9 +498,9 @@ static int _dpu_rm_populate_requirements( { reqs->topology = req_topology; - DRM_DEBUG_KMS("num_lm: %d num_dsc: %d num_intf: %d\n", + DRM_DEBUG_KMS("num_lm: %d num_dsc: %d num_intf: %d cdm: %d\n", reqs->topology.num_lm, reqs->topology.num_dsc, - reqs->topology.num_intf); + reqs->topology.num_intf, reqs->topology.needs_cdm); return 0; } @@ -501,6 +529,7 @@ void dpu_rm_release(struct dpu_global_state *global_state, ARRAY_SIZE(global_state->dsc_to_enc_id), enc->base.id); _dpu_rm_clear_mapping(global_state->dspp_to_enc_id, ARRAY_SIZE(global_state->dspp_to_enc_id), enc->base.id); + _dpu_rm_clear_mapping(_state->cdm_to_enc_id, 1, enc->base.id); } int dpu_rm_reserve( @@ -574,6 +603,11 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm, hw_to_enc_id = global_state->dsc_to_enc_id; max_blks = ARRAY_SIZE(rm->dsc_blks); break; + case DPU_HW_BLK_CDM: + hw_blks = >cdm_blk; + hw_to_enc_id = _state->cdm_to_enc_id; + max_blks = 1; + break; default: DPU_ERROR("blk type %d not managed by rm\n", type); return 0; diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index c0446fa66b98..16a7cbc0b7dd 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -90,12 +90,14 @@ enum msm_event_wait { * @num_intf: number of interfaces the panel is mounted on * @num_dspp: number of dspp blocks used * @num_dsc: number of Display Stream Compression (DSC) blocks used + * @needs_cdm:indicates whether cdm block is needed for this display topology */ struct msm_display_topology { u32 num_lm; u32 num_intf; u32 num_dspp; u32 num_dsc; + bool needs_cdm; }; /* Commit/Event thread specific structure */ -- 2.40.1
[PATCH v3 08/15] drm/msm/dpu: add cdm blocks to RM
Add the RM APIs necessary to initialize and allocate CDM blocks to be used by the rest of the DPU pipeline. changes in v2: - treat cdm_init() failure as fatal - fixed the commit text Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 ++ 2 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 0bb28cf4a6cb..7ed476b96304 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -8,6 +8,7 @@ #include "dpu_kms.h" #include "dpu_hw_lm.h" #include "dpu_hw_ctl.h" +#include "dpu_hw_cdm.h" #include "dpu_hw_pingpong.h" #include "dpu_hw_sspp.h" #include "dpu_hw_intf.h" @@ -176,6 +177,18 @@ int dpu_rm_init(struct drm_device *dev, rm->hw_sspp[sspp->id - SSPP_NONE] = hw; } + if (cat->cdm) { + struct dpu_hw_cdm *hw; + + hw = dpu_hw_cdm_init(dev, cat->cdm, mmio, cat->mdss_ver); + if (IS_ERR(hw)) { + rc = PTR_ERR(hw); + DPU_ERROR("failed cdm object creation: err %d\n", rc); + goto fail; + } + rm->cdm_blk = >base; + } + return 0; fail: diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h index 36752d837be4..e3f83ebc656b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h @@ -22,6 +22,7 @@ struct dpu_global_state; * @hw_wb: array of wb hardware resources * @dspp_blks: array of dspp hardware resources * @hw_sspp: array of sspp hardware resources + * @cdm_blk: cdm hardware resource */ struct dpu_rm { struct dpu_hw_blk *pingpong_blks[PINGPONG_MAX - PINGPONG_0]; @@ -33,6 +34,7 @@ struct dpu_rm { struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0]; struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0]; struct dpu_hw_sspp *hw_sspp[SSPP_MAX - SSPP_NONE]; + struct dpu_hw_blk *cdm_blk; }; /** -- 2.40.1
[PATCH v3 04/15] drm/msm/dpu: move csc matrices to dpu_hw_util
Since the type and usage of CSC matrices is spanning across DPU lets introduce a helper to the dpu_hw_util to return the CSC corresponding to the request type. This will help to add more supported CSC types such as the RGB to YUV one which is used in the case of CDM. changes in v3: - drop the extra wrapper and export the matrices directly Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 30 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 + 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h index fe083b2e5696..aa50005042d1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h @@ -19,6 +19,36 @@ #define MISR_CTRL_STATUS_CLEAR BIT(10) #define MISR_CTRL_FREE_RUN_MASK BIT(31) +static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = { + { + /* S15.16 format */ + 0x00012A00, 0x, 0x00019880, + 0x00012A00, 0x9B80, 0x3000, + 0x00012A00, 0x00020480, 0x, + }, + /* signed bias */ + { 0xfff0, 0xff80, 0xff80,}, + { 0x0, 0x0, 0x0,}, + /* unsigned clamp */ + { 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,}, + { 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,}, +}; + +static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = { + { + /* S15.16 format */ + 0x00012A00, 0x, 0x00019880, + 0x00012A00, 0x9B80, 0x3000, + 0x00012A00, 0x00020480, 0x, + }, + /* signed bias */ + { 0xffc0, 0xfe00, 0xfe00,}, + { 0x0, 0x0, 0x0,}, + /* unsigned clamp */ + { 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,}, + { 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,}, +}; + /* * This is the common struct maintained by each sub block * for mapping the register offsets in this block to the diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 3235ab132540..ff975ad51145 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -21,6 +21,7 @@ #include "dpu_kms.h" #include "dpu_formats.h" #include "dpu_hw_sspp.h" +#include "dpu_hw_util.h" #include "dpu_trace.h" #include "dpu_crtc.h" #include "dpu_vbif.h" @@ -508,36 +509,6 @@ static void _dpu_plane_setup_pixel_ext(struct dpu_hw_scaler3_cfg *scale_cfg, } } -static const struct dpu_csc_cfg dpu_csc_YUV2RGB_601L = { - { - /* S15.16 format */ - 0x00012A00, 0x, 0x00019880, - 0x00012A00, 0x9B80, 0x3000, - 0x00012A00, 0x00020480, 0x, - }, - /* signed bias */ - { 0xfff0, 0xff80, 0xff80,}, - { 0x0, 0x0, 0x0,}, - /* unsigned clamp */ - { 0x10, 0xeb, 0x10, 0xf0, 0x10, 0xf0,}, - { 0x00, 0xff, 0x00, 0xff, 0x00, 0xff,}, -}; - -static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = { - { - /* S15.16 format */ - 0x00012A00, 0x, 0x00019880, - 0x00012A00, 0x9B80, 0x3000, - 0x00012A00, 0x00020480, 0x, - }, - /* signed bias */ - { 0xffc0, 0xfe00, 0xfe00,}, - { 0x0, 0x0, 0x0,}, - /* unsigned clamp */ - { 0x40, 0x3ac, 0x40, 0x3c0, 0x40, 0x3c0,}, - { 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,}, -}; - static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_sw_pipe *pipe, const struct dpu_format *fmt) { -- 2.40.1
[PATCH v3 02/15] drm/msm/dpu: rename dpu_encoder_phys_wb_setup_cdp to match its functionality
dpu_encoder_phys_wb_setup_cdp() is not programming the chroma down prefetch block. Its setting up the display ctl path for writeback. Hence rename it to dpu_encoder_phys_wb_setup_ctl() to match what its actually doing. Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback") Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index 425415d45ec1..8f05f2a1fc24 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -207,10 +207,10 @@ static void dpu_encoder_phys_wb_setup_fb(struct dpu_encoder_phys *phys_enc, } /** - * dpu_encoder_phys_wb_setup_cdp - setup chroma down prefetch block + * dpu_encoder_phys_wb_setup_ctl - setup wb pipeline for ctl path * @phys_enc:Pointer to physical encoder */ -static void dpu_encoder_phys_wb_setup_cdp(struct dpu_encoder_phys *phys_enc) +static void dpu_encoder_phys_wb_setup_ctl(struct dpu_encoder_phys *phys_enc) { struct dpu_hw_wb *hw_wb; struct dpu_hw_ctl *ctl; @@ -382,7 +382,7 @@ static void dpu_encoder_phys_wb_setup( dpu_encoder_phys_wb_setup_fb(phys_enc, fb); - dpu_encoder_phys_wb_setup_cdp(phys_enc); + dpu_encoder_phys_wb_setup_ctl(phys_enc); } -- 2.40.1
[PATCH v3 05/15] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
Add CDM blocks to the sc7280 dpu_hw_catalog to support YUV format output from writeback block. changes in v3: - change the comment from sub-blk to clk for CDM changes in v2: - remove explicit zero assignment for features - move sc7280_cdm to dpu_hw_catalog from the sc7280 catalog file as its definition can be re-used Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 + 4 files changed, 29 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 209675de6742..19c2b7454796 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { .mdss_ver = _mdss_ver, .caps = _dpu_caps, .mdp = _mdp, + .cdm = _cdm, .ctl_count = ARRAY_SIZE(sc7280_ctl), .ctl = sc7280_ctl, .sspp_count = ARRAY_SIZE(sc7280_sspp), 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 d52aae54bbd5..b304bebedb84 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, }; +/* + * CDM block config + */ +static const struct dpu_cdm_cfg sc7280_cdm = { + .name = "cdm_0", + .id = CDM_0, + .len = 0x228, + .base = 0x79200, +}; + /* * VBIF sub blocks config */ 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 e3c0d007481b..ba82ef4560a6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { u32 memtype[MAX_XIN_COUNT]; }; +/** + * struct dpu_cdm_cfg - information of chroma down blocks + * @name string name for debug purposes + * @id enum identifying this block + * @base register offset of this block + * @features bit mask identifying sub-blocks/features + */ +struct dpu_cdm_cfg { + DPU_HW_BLK_INFO; +}; + /** * Define CDP use cases * @DPU_PERF_CDP_UDAGE_RT: real-time use cases @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { u32 wb_count; const struct dpu_wb_cfg *wb; + const struct dpu_cdm_cfg *cdm; + u32 ad_count; u32 dspp_count; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h index a6702b2bfc68..f319c8232ea5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h @@ -185,6 +185,11 @@ enum dpu_dsc { DSC_MAX }; +enum dpu_cdm { + CDM_0 = 1, + CDM_MAX +}; + enum dpu_pingpong { PINGPONG_NONE, PINGPONG_0, -- 2.40.1
[PATCH v3 03/15] drm/msm/dpu: fix writeback programming for YUV cases
For YUV cases, setting the required format bits was missed out in the register programming. Lets fix it now in preparation of adding YUV formats support for writeback. changes in v2: - dropped the fixes tag as its not a fix but adding new functionality Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c index ed0e80616129..e75995f7fcea 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c @@ -89,6 +89,9 @@ static void dpu_hw_wb_setup_format(struct dpu_hw_wb *ctx, dst_format |= BIT(14); /* DST_ALPHA_X */ } + if (DPU_FORMAT_IS_YUV(fmt)) + dst_format |= BIT(15); + pattern = (fmt->element[3] << 24) | (fmt->element[2] << 16) | (fmt->element[1] << 8) | -- 2.40.1
[PATCH v3 01/15] drm/msm/dpu: add formats check for writeback encoder
In preparation for adding more formats to dpu writeback add format validation to it to fail any unsupported formats. changes in v3: - rebase on top of msm-next - replace drm_atomic_helper_check_wb_encoder_state() with drm_atomic_helper_check_wb_connector_state() due to the rebase changes in v2: - correct some grammar in the commit text Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback") Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index bb94909caa25..425415d45ec1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -272,6 +272,7 @@ static int dpu_encoder_phys_wb_atomic_check( { struct drm_framebuffer *fb; const struct drm_display_mode *mode = _state->mode; + int ret; DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n", phys_enc->hw_wb->idx, mode->name, mode->hdisplay, mode->vdisplay); @@ -308,6 +309,12 @@ static int dpu_encoder_phys_wb_atomic_check( return -EINVAL; } + ret = drm_atomic_helper_check_wb_connector_state(conn_state->connector, conn_state->state); + if (ret < 0) { + DPU_ERROR("invalid pixel format %p4cc\n", >format->format); + return ret; + } + return 0; } -- 2.40.1
[PATCH v3 00/15] Add CDM support for MSM writeback
Chroma Down Sampling (CDM) block is a hardware block in the DPU pipeline which among other things has a CSC block that can convert RGB input from the DPU to YUV data. This block can be used with either HDMI, DP or writeback interface. In this series, lets first add the support for CDM block to be used with writeback and then follow-up with support for other interfaces such as DP. This was validated by adding support to pass custom output format to the IGT's kms_writeback test-case, specifically only for the output dump test-case [1]. The usage for this is: ./kms_writeback -d -f So for NV12, this can be verified with the below command: ./kms_writeback -d -f NV12 [1] : https://patchwork.freedesktop.org/series/122125/ changes in v3: - rebase on top of msm-next - drop the extra wrapper and export the CSC matrices directly - fixes in commit text as requested - fixes for kbot errors as reported - drop "enable" parameter from bind_pingpong_blk() as we can just use PINGPONG_NONE for disable cases - squash cdm changes in encoder cleanup to the change which allocates cdm changes in v2: - rebased on top of current msm-next-lumag - fix commit text of some of the patches - move csc matrices to dpu_hw_util as they span across DPU - move cdm blk define to dpu_hw_catalog as its common across chipsets - remove bit magic in dpu_hw_cdm with relevant defines - use drmm_kzalloc instead of kzalloc/free - protect bind_pingpong_blk with core_rev check - drop setup_csc_data() and setup_cdwn() ops as they are merged into enable() - protect bind_pingpong_blk with core_rev check - drop setup_csc_data() and setup_cdwn() ops as they are merged into enable() - move needs_cdm to topology struct - call update_pending_flush_cdm even when bind_pingpong_blk is not present - drop usage of setup_csc_data() and setup_cdwn() cdm ops as they both have been merged into enable() - drop reduntant hw_cdm and hw_pp checks - drop fb related checks from dpu_encoder::atomic_mode_set() - introduce separate wb2_format arrays for rgb and yuv Abhinav Kumar (15): drm/msm/dpu: add formats check for writeback encoder drm/msm/dpu: rename dpu_encoder_phys_wb_setup_cdp to match its functionality drm/msm/dpu: fix writeback programming for YUV cases drm/msm/dpu: move csc matrices to dpu_hw_util drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog drm/msm/dpu: add cdm blocks to sm8250 dpu_hw_catalog drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block drm/msm/dpu: add cdm blocks to RM drm/msm/dpu: add support to allocate CDM from RM drm/msm/dpu: add CDM related logic to dpu_hw_ctl layer drm/msm/dpu: add an API to setup the CDM block for writeback drm/msm/dpu: plug-in the cdm related bits to writeback setup drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output drm/msm/dpu: introduce separate wb2_format arrays for rgb and yuv drm/msm/dpu: add cdm blocks to dpu snapshot drivers/gpu/drm/msm/Makefile | 1 + .../msm/disp/dpu1/catalog/dpu_10_0_sm8650.h | 4 +- .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h| 5 +- .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h| 4 +- .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h| 5 +- .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 37 +++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 + .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 114 +++- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 47 +++- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c| 263 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h| 130 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 33 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h| 12 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 7 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 44 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 51 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h| 2 + drivers/gpu/drm/msm/msm_drv.h | 2 + 24 files changed, 777 insertions(+), 46 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h -- 2.40.1
Re: [PATCH v3] drm/msm/dpu: improve DSC allocation
On 12/11/2023 1:30 PM, Dmitry Baryshkov wrote: On Mon, 11 Dec 2023 at 20:38, Kuogee Hsieh wrote: A DCE (Display Compression Engine) contains two DSC hard slice encoders. Each DCE start with even DSC encoder index followed by "starts". But it will not be correct. The DCE doesn't start with the DSC encoder. DCE consists of two DSC encoders, one has an odd index and another one has an even index. an odd DSC encoder index. Each encoder can work independently. But Only two DSC encoders from same DCE can be paired to work only together to support merge mode. In addition, the DSC with even There are different merge modes. Here you are talking about the DSC merge mode. index have to mapping to even pingpong index and DSC with odd PINGPONG (end everywhere else). have to be mapped, should be used, etc. index have to mapping to odd pingpong index at its data path. This patch improve DSC allocation mechanism with consideration improves of above factors. of these factors. Changes in V3: -- add dpu_rm_pingpong_dsc_check() -- for pair allocation use i += 2 at for loop Changes in V2: -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and _dpu_rm_reserve_dsc_pair() Fixes: f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in RM") This tag is incorrect. The patch should be split into two pieces. One which fixes DSC allocation for DSC 1.1 encoders, where there were no DCE blocks, another one which adds proper handling for DCE. Unless the paired allocation requirement also applies to pre-DCE DSC encoders. But in that case the commit message doesn't make any sense. I checked 4.x Qualcomm kernels. None of them contained any of these restrictions for DSC blocks. Only the displaypack targeting 4.19 kernel got these changes. But it predates DCE pairs support. as I said earlier the rule of odd/even pp-index map to odd/even dsc-index is there since dsc v1.1. I think current code (including down stream code) works by luck to not encounter a configuration with two independence paths, one with single dsc and the other one use two dsc to support dsc merge mode. this patch is the fix to enforce this rule for both dsc v1.1 and v1.2 and I will rework commit message yo have better description. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 171 ++--- 1 file changed, 155 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 17ecf23..43598ee 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -470,33 +470,172 @@ static int _dpu_rm_reserve_ctls( return 0; } -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm, +static int _dpu_rm_pingpong_dsc_check(int dsc_idx, + uint32_t enc_id, + uint32_t *pp_to_enc_id, + int pp_max, + bool pair) +{ + int pp_idx; + + /* +* find the pingpong index which had been reserved +* previously at layer mixer allocation during +*/ + for (pp_idx = 0; pp_idx < pp_max; pp_idx++) { + if (pp_to_enc_id[pp_idx] == enc_id) + break; + } + + /* +* dsc even index must map to pingpong even index DSC with even index. be mapped or correspond +* dsc odd index must map to pingpong odd index +*/ + if ((dsc_idx & 0x01) != (pp_idx & 0x01)) + return -ENAVAIL; + + if (pair) { + /* +* delete pp_idx so that same pp_id can not be paired with +* next dsc_id +*/ + pp_to_enc_id[pp_idx] = 0x; + } Ugh. "Non tangere circulos meos". Move this deletion away from this function. + + return 0; + +} + +static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm, struct dpu_global_state *global_state, - struct drm_encoder *enc, + uint32_t enc_id, const struct msm_display_topology *top) { - int num_dsc = top->num_dsc; - int i; + int num_dsc = 0; + int i, ret; + int dsc_id[DSC_MAX - DSC_0]; + uint32_t *pp_enc_id = global_state->pingpong_to_enc_id; + int pp_max = PINGPONG_MAX - PINGPONG_0; - /* check if DSC required are allocated or not */ - for (i = 0; i < num_dsc; i++) { - if (!rm->dsc_blks[i]) { - DPU_ERROR("DSC %d does not exist\n", i); - return -EIO; - } + memset(dsc_id, 0, sizeof(dsc_id)); - if (global_state->dsc_to_enc_id[i]) { - DPU_ERROR("DSC %d is already allocated\n", i); - return -EIO; -
Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
On Mon, Dec 11, 2023 at 2:09 PM Marijn Suijten wrote: > > On 2023-12-11 10:19:55, Rob Clark wrote: > > From: Rob Clark > > > > When we start getting these, we get a *lot*. So ratelimit it to not > > flood dmesg. > > > > Signed-off-by: Rob Clark > > --- > > > > dpu should probably stop rolling it's own trace macros, but that would > > be a larger cleanup. > > That would be lovely, use is currently all over the place. > > Should this patch also ratelimit the corresponding: > > [drm:dpu_encoder_phys_cmd_prepare_for_kickoff] *ERROR* failed > wait_for_idle: id:31 ret:-110 pp:0 > > On CMD-mode panels? Probably it should for consistency. But I think you normally wouldn't get this error at 60Hz with a cmd mode panel, so probably ok to make it ratelimited for cmd mode later. BR, -R > Note that this is a prime example of using DRM_ERROR over DPU_ERROR*, > resulting > in unnecessary divergence (and un-readability) between error messages and the > code (DPU_DEBUG_CMDENC, which has a corresponding DPU_ERROR variant, is also > used within that function...) > > Reviewed-by: Marijn Suijten > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 - > > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > index 82538844614b..7c22235d0eba 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > @@ -39,6 +39,9 @@ > > #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\ > > (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) > > > > +#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) > > DPU_ERROR_RATELIMITED("enc%d " fmt,\ > > + (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) > > + > > /* > > * Two to anticipate panels that can do cmd/vid dynamic switching > > * plan is to create all possible physical encoder types, and switch > > between > > @@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct > > timer_list *t) > > return; > > } > > > > - DPU_ERROR_ENC(dpu_enc, "frame done timeout\n"); > > + DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n"); > > > > event = DPU_ENCODER_FRAME_EVENT_ERROR; > > trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event); > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > > index b6f53ca6e962..f5473d4dea92 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > > @@ -51,6 +51,7 @@ > > } while (0) > > > > #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__) > > +#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" > > fmt, ##__VA_ARGS__) > > > > /** > > * ktime_compare_safe - compare two ktime structures > > -- > > 2.43.0 > >
Re: [PATCH v2] iommu/arm-smmu-qcom: Add missing GMU entry to match table
On 2023-12-10 6:06 pm, Rob Clark wrote: From: Rob Clark In some cases the firmware expects cbndx 1 to be assigned to the GMU, so we also want the default domain for the GMU to be an identy domain. This way it does not get a context bank assigned. Without this, both of_dma_configure() and drm/msm's iommu_domain_attach() will trigger allocating and configuring a context bank. So GMU ends up attached to both cbndx 1 and later cbndx 2. This arrangement seemingly confounds and surprises the firmware if the GPU later triggers a translation fault, resulting (on sc8280xp / lenovo x13s, at least) in the SMMU getting wedged and the GPU stuck without memory access. Reviewed-by: Robin Murphy Cc: sta...@vger.kernel.org Signed-off-by: Rob Clark --- I didn't add a fixes tag because really this issue has been there all along, but either didn't matter with other firmware or we didn't notice the problem. drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 549ae4dba3a6..d326fa230b96 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -243,6 +243,7 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain, static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = { { .compatible = "qcom,adreno" }, + { .compatible = "qcom,adreno-gmu" }, { .compatible = "qcom,mdp4" }, { .compatible = "qcom,mdss" }, { .compatible = "qcom,sc7180-mdss" },
Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
On 2023-12-11 10:19:55, Rob Clark wrote: > From: Rob Clark > > When we start getting these, we get a *lot*. So ratelimit it to not > flood dmesg. > > Signed-off-by: Rob Clark > --- > > dpu should probably stop rolling it's own trace macros, but that would > be a larger cleanup. That would be lovely, use is currently all over the place. Should this patch also ratelimit the corresponding: [drm:dpu_encoder_phys_cmd_prepare_for_kickoff] *ERROR* failed wait_for_idle: id:31 ret:-110 pp:0 On CMD-mode panels? Note that this is a prime example of using DRM_ERROR over DPU_ERROR*, resulting in unnecessary divergence (and un-readability) between error messages and the code (DPU_DEBUG_CMDENC, which has a corresponding DPU_ERROR variant, is also used within that function...) Reviewed-by: Marijn Suijten > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 - > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 82538844614b..7c22235d0eba 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -39,6 +39,9 @@ > #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\ > (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) > > +#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d > " fmt,\ > + (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) > + > /* > * Two to anticipate panels that can do cmd/vid dynamic switching > * plan is to create all possible physical encoder types, and switch between > @@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct > timer_list *t) > return; > } > > - DPU_ERROR_ENC(dpu_enc, "frame done timeout\n"); > + DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n"); > > event = DPU_ENCODER_FRAME_EVENT_ERROR; > trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event); > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > index b6f53ca6e962..f5473d4dea92 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h > @@ -51,6 +51,7 @@ > } while (0) > > #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__) > +#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" > fmt, ##__VA_ARGS__) > > /** > * ktime_compare_safe - compare two ktime structures > -- > 2.43.0 >
Re: [PATCH 1/9] dt-bindings: display: msm: dp: declare compatible string for sm8150
On 10/12/2023 00:21, Dmitry Baryshkov wrote: > Add compatible string for the DisplayPort controller found on the > Qualcomm SM8150 platform. > > Signed-off-by: Dmitry Baryshkov > --- Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 05/16] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
On 12/11/2023 1:42 PM, Dmitry Baryshkov wrote: On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar wrote: On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar wrote: On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar wrote: Add CDM blocks to the sc7280 dpu_hw_catalog to support YUV format output from writeback block. changes in v2: - remove explicit zero assignment for features - move sc7280_cdm to dpu_hw_catalog from the sc7280 catalog file as its definition can be re-used Signed-off-by: Abhinav Kumar --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 + 4 files changed, 29 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 209675de6742..19c2b7454796 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { .mdss_ver = _mdss_ver, .caps = _dpu_caps, .mdp = _mdp, + .cdm = _cdm, .ctl_count = ARRAY_SIZE(sc7280_ctl), .ctl = sc7280_ctl, .sspp_count = ARRAY_SIZE(sc7280_sspp), 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 d52aae54bbd5..1be3156cde05 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, }; +/* + * CDM sub block config Nit: it is not a subblock config. Ack. + */ +static const struct dpu_cdm_cfg sc7280_cdm = { I know that I have r-b'ed this patch. But then one thing occurred to me. If this definition is common to all (or almost all) platforms, can we just call it dpu_cdm or dpu_common_cdm? + .name = "cdm_0", + .id = CDM_0, + .len = 0x228, + .base = 0x79200, +}; hmmm almost common but not entirely ... msm8998's CDM has a shorter len of 0x224 :( Then sdm845_cdm? That also has a shorter cdm length. Could you please clarify. According to the downstream DT files all CDM blocks up to (but not including) sm8550 had length 0x224. SM8550 and SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers after 0x204. We always list 0x4 more than the last offset. In chipsets sdm845 and msm8998, I only see the last offset of CDM as 0x220 but in sm8250 and sc7280, the last offset is 0x224. Hence the total length is more in sc7280/sm8250 as compared to sdm845/msm8998. I didnt follow that you do not see any registers after 0x204. The CDM_MUX is the last offset which has an offset 0x224 from the base address. So thats the last offset. The newer chipsets have CDM_MUX and the older ones did not. Hence the difference in length. BTW, sdm845 is not in this series. It will be part of RFT as we discussed. + /* * VBIF sub blocks config */ 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 e3c0d007481b..ba82ef4560a6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { u32 memtype[MAX_XIN_COUNT]; }; +/** + * struct dpu_cdm_cfg - information of chroma down blocks + * @name string name for debug purposes + * @id enum identifying this block + * @base register offset of this block + * @features bit mask identifying sub-blocks/features + */ +struct dpu_cdm_cfg { + DPU_HW_BLK_INFO; +}; + /** * Define CDP use cases * @DPU_PERF_CDP_UDAGE_RT: real-time use cases @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { u32 wb_count; const struct dpu_wb_cfg *wb; + const struct dpu_cdm_cfg *cdm; + u32 ad_count; u32 dspp_count; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h index a6702b2bfc68..f319c8232ea5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h @@ -185,6 +185,11 @@ enum dpu_dsc { DSC_MAX }; +enum dpu_cdm { + CDM_0 = 1, + CDM_MAX +}; + enum dpu_pingpong {
Re: [PATCH v2 05/16] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
On Mon, 11 Dec 2023 at 23:32, Abhinav Kumar wrote: > > > > On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: > > On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar > > wrote: > >> > >> > >> > >> On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > >>> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar > >>> wrote: > > Add CDM blocks to the sc7280 dpu_hw_catalog to support > YUV format output from writeback block. > > changes in v2: > - remove explicit zero assignment for features > - move sc7280_cdm to dpu_hw_catalog from the sc7280 > catalog file as its definition can be re-used > > Signed-off-by: Abhinav Kumar > --- > .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 + > 4 files changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > index 209675de6742..19c2b7454796 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > .mdss_ver = _mdss_ver, > .caps = _dpu_caps, > .mdp = _mdp, > + .cdm = _cdm, > .ctl_count = ARRAY_SIZE(sc7280_ctl), > .ctl = sc7280_ctl, > .sspp_count = ARRAY_SIZE(sc7280_sspp), > 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 d52aae54bbd5..1be3156cde05 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { > .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > }; > > +/* > + * CDM sub block config > >>> > >>> Nit: it is not a subblock config. > >>> > >> > >> Ack. > >> > + */ > +static const struct dpu_cdm_cfg sc7280_cdm = { > >>> > >>> I know that I have r-b'ed this patch. But then one thing occurred to > >>> me. If this definition is common to all (or almost all) platforms, can > >>> we just call it dpu_cdm or dpu_common_cdm? > >>> > + .name = "cdm_0", > + .id = CDM_0, > + .len = 0x228, > + .base = 0x79200, > +}; > >> > >> hmmm almost common but not entirely ... msm8998's CDM has a shorter > >> len of 0x224 :( > > > > Then sdm845_cdm? > > > > That also has a shorter cdm length. Could you please clarify. According to the downstream DT files all CDM blocks up to (but not including) sm8550 had length 0x224. SM8550 and SM8650 got qcom,sde-cdm-size = <0x220>. But I don't see any registers after 0x204. > > BTW, sdm845 is not in this series. It will be part of RFT as we discussed. > > >> > + > /* > * VBIF sub blocks config > */ > 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 e3c0d007481b..ba82ef4560a6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > u32 memtype[MAX_XIN_COUNT]; > }; > > +/** > + * struct dpu_cdm_cfg - information of chroma down blocks > + * @name string name for debug purposes > + * @id enum identifying this block > + * @base register offset of this block > + * @features bit mask identifying sub-blocks/features > + */ > +struct dpu_cdm_cfg { > + DPU_HW_BLK_INFO; > +}; > + > /** > * Define CDP use cases > * @DPU_PERF_CDP_UDAGE_RT: real-time use cases > @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { > u32 wb_count; > const struct dpu_wb_cfg *wb; > > + const struct dpu_cdm_cfg *cdm; > + > u32 ad_count; > > u32 dspp_count; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > index a6702b2bfc68..f319c8232ea5 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > @@ -185,6
Re: [PATCH v2 05/16] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
On 12/11/2023 1:31 PM, Dmitry Baryshkov wrote: On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar wrote: On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar wrote: Add CDM blocks to the sc7280 dpu_hw_catalog to support YUV format output from writeback block. changes in v2: - remove explicit zero assignment for features - move sc7280_cdm to dpu_hw_catalog from the sc7280 catalog file as its definition can be re-used Signed-off-by: Abhinav Kumar --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 + 4 files changed, 29 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 209675de6742..19c2b7454796 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { .mdss_ver = _mdss_ver, .caps = _dpu_caps, .mdp = _mdp, + .cdm = _cdm, .ctl_count = ARRAY_SIZE(sc7280_ctl), .ctl = sc7280_ctl, .sspp_count = ARRAY_SIZE(sc7280_sspp), 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 d52aae54bbd5..1be3156cde05 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, }; +/* + * CDM sub block config Nit: it is not a subblock config. Ack. + */ +static const struct dpu_cdm_cfg sc7280_cdm = { I know that I have r-b'ed this patch. But then one thing occurred to me. If this definition is common to all (or almost all) platforms, can we just call it dpu_cdm or dpu_common_cdm? + .name = "cdm_0", + .id = CDM_0, + .len = 0x228, + .base = 0x79200, +}; hmmm almost common but not entirely ... msm8998's CDM has a shorter len of 0x224 :( Then sdm845_cdm? That also has a shorter cdm length. BTW, sdm845 is not in this series. It will be part of RFT as we discussed. + /* * VBIF sub blocks config */ 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 e3c0d007481b..ba82ef4560a6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { u32 memtype[MAX_XIN_COUNT]; }; +/** + * struct dpu_cdm_cfg - information of chroma down blocks + * @name string name for debug purposes + * @id enum identifying this block + * @base register offset of this block + * @features bit mask identifying sub-blocks/features + */ +struct dpu_cdm_cfg { + DPU_HW_BLK_INFO; +}; + /** * Define CDP use cases * @DPU_PERF_CDP_UDAGE_RT: real-time use cases @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { u32 wb_count; const struct dpu_wb_cfg *wb; + const struct dpu_cdm_cfg *cdm; + u32 ad_count; u32 dspp_count; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h index a6702b2bfc68..f319c8232ea5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h @@ -185,6 +185,11 @@ enum dpu_dsc { DSC_MAX }; +enum dpu_cdm { + CDM_0 = 1, + CDM_MAX +}; + enum dpu_pingpong { PINGPONG_NONE, PINGPONG_0, -- 2.40.1
Re: [PATCH v2 05/16] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
On Mon, 11 Dec 2023 at 23:16, Abhinav Kumar wrote: > > > > On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: > > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar > > wrote: > >> > >> Add CDM blocks to the sc7280 dpu_hw_catalog to support > >> YUV format output from writeback block. > >> > >> changes in v2: > >> - remove explicit zero assignment for features > >> - move sc7280_cdm to dpu_hw_catalog from the sc7280 > >>catalog file as its definition can be re-used > >> > >> Signed-off-by: Abhinav Kumar > >> --- > >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 + > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 + > >> 4 files changed, 29 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> index 209675de6742..19c2b7454796 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h > >> @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { > >> .mdss_ver = _mdss_ver, > >> .caps = _dpu_caps, > >> .mdp = _mdp, > >> + .cdm = _cdm, > >> .ctl_count = ARRAY_SIZE(sc7280_ctl), > >> .ctl = sc7280_ctl, > >> .sspp_count = ARRAY_SIZE(sc7280_sspp), > >> 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 d52aae54bbd5..1be3156cde05 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >> @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { > >> .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, > >> }; > >> > >> +/* > >> + * CDM sub block config > > > > Nit: it is not a subblock config. > > > > Ack. > > >> + */ > >> +static const struct dpu_cdm_cfg sc7280_cdm = { > > > > I know that I have r-b'ed this patch. But then one thing occurred to > > me. If this definition is common to all (or almost all) platforms, can > > we just call it dpu_cdm or dpu_common_cdm? > > > >> + .name = "cdm_0", > >> + .id = CDM_0, > >> + .len = 0x228, > >> + .base = 0x79200, > >> +}; > > hmmm almost common but not entirely ... msm8998's CDM has a shorter > len of 0x224 :( Then sdm845_cdm? > > >> + > >> /* > >>* VBIF sub blocks config > >>*/ > >> 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 e3c0d007481b..ba82ef4560a6 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h > >> @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { > >> u32 memtype[MAX_XIN_COUNT]; > >> }; > >> > >> +/** > >> + * struct dpu_cdm_cfg - information of chroma down blocks > >> + * @name string name for debug purposes > >> + * @id enum identifying this block > >> + * @base register offset of this block > >> + * @features bit mask identifying sub-blocks/features > >> + */ > >> +struct dpu_cdm_cfg { > >> + DPU_HW_BLK_INFO; > >> +}; > >> + > >> /** > >>* Define CDP use cases > >>* @DPU_PERF_CDP_UDAGE_RT: real-time use cases > >> @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { > >> u32 wb_count; > >> const struct dpu_wb_cfg *wb; > >> > >> + const struct dpu_cdm_cfg *cdm; > >> + > >> u32 ad_count; > >> > >> u32 dspp_count; > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >> index a6702b2bfc68..f319c8232ea5 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h > >> @@ -185,6 +185,11 @@ enum dpu_dsc { > >> DSC_MAX > >> }; > >> > >> +enum dpu_cdm { > >> + CDM_0 = 1, > >> + CDM_MAX > >> +}; > >> + > >> enum dpu_pingpong { > >> PINGPONG_NONE, > >> PINGPONG_0, > >> -- > >> 2.40.1 > >> > > > > -- With best wishes Dmitry
Re: [PATCH v3] drm/msm/dpu: improve DSC allocation
On Mon, 11 Dec 2023 at 20:38, Kuogee Hsieh wrote: > > A DCE (Display Compression Engine) contains two DSC hard slice > encoders. Each DCE start with even DSC encoder index followed by "starts". But it will not be correct. The DCE doesn't start with the DSC encoder. DCE consists of two DSC encoders, one has an odd index and another one has an even index. > an odd DSC encoder index. Each encoder can work independently. > But Only two DSC encoders from same DCE can be paired to work only > together to support merge mode. In addition, the DSC with even There are different merge modes. Here you are talking about the DSC merge mode. > index have to mapping to even pingpong index and DSC with odd PINGPONG (end everywhere else). have to be mapped, should be used, etc. > index have to mapping to odd pingpong index at its data path. > This patch improve DSC allocation mechanism with consideration improves > of above factors. of these factors. > > Changes in V3: > -- add dpu_rm_pingpong_dsc_check() > -- for pair allocation use i += 2 at for loop > > Changes in V2: > -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and >_dpu_rm_reserve_dsc_pair() > > Fixes: f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in RM") This tag is incorrect. The patch should be split into two pieces. One which fixes DSC allocation for DSC 1.1 encoders, where there were no DCE blocks, another one which adds proper handling for DCE. Unless the paired allocation requirement also applies to pre-DCE DSC encoders. But in that case the commit message doesn't make any sense. I checked 4.x Qualcomm kernels. None of them contained any of these restrictions for DSC blocks. Only the displaypack targeting 4.19 kernel got these changes. But it predates DCE pairs support. > Signed-off-by: Kuogee Hsieh > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 171 > ++--- > 1 file changed, 155 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > index 17ecf23..43598ee 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > @@ -470,33 +470,172 @@ static int _dpu_rm_reserve_ctls( > return 0; > } > > -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm, > +static int _dpu_rm_pingpong_dsc_check(int dsc_idx, > + uint32_t enc_id, > + uint32_t *pp_to_enc_id, > + int pp_max, > + bool pair) > +{ > + int pp_idx; > + > + /* > +* find the pingpong index which had been reserved > +* previously at layer mixer allocation during > +*/ > + for (pp_idx = 0; pp_idx < pp_max; pp_idx++) { > + if (pp_to_enc_id[pp_idx] == enc_id) > + break; > + } > + > + /* > +* dsc even index must map to pingpong even index DSC with even index. be mapped or correspond > +* dsc odd index must map to pingpong odd index > +*/ > + if ((dsc_idx & 0x01) != (pp_idx & 0x01)) > + return -ENAVAIL; > + > + if (pair) { > + /* > +* delete pp_idx so that same pp_id can not be paired with > +* next dsc_id > +*/ > + pp_to_enc_id[pp_idx] = 0x; > + } Ugh. "Non tangere circulos meos". Move this deletion away from this function. > + > + return 0; > + > +} > + > +static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm, >struct dpu_global_state *global_state, > - struct drm_encoder *enc, > + uint32_t enc_id, >const struct msm_display_topology *top) > { > - int num_dsc = top->num_dsc; > - int i; > + int num_dsc = 0; > + int i, ret; > + int dsc_id[DSC_MAX - DSC_0]; > + uint32_t *pp_enc_id = global_state->pingpong_to_enc_id; > + int pp_max = PINGPONG_MAX - PINGPONG_0; > > - /* check if DSC required are allocated or not */ > - for (i = 0; i < num_dsc; i++) { > - if (!rm->dsc_blks[i]) { > - DPU_ERROR("DSC %d does not exist\n", i); > - return -EIO; > - } > + memset(dsc_id, 0, sizeof(dsc_id)); > > - if (global_state->dsc_to_enc_id[i]) { > - DPU_ERROR("DSC %d is already allocated\n", i); > - return -EIO; > - } > + for (i = 0; i < ARRAY_SIZE(rm->dsc_blks) && num_dsc >= top->num_dsc; > i++) { Wait. num_dsc >= top->num_dsc? num_dsc starts with 0, so this loop will never be executed? > + if (!rm->dsc_blks[i]) > + continue; > + > + if (global_state->dsc_to_enc_id[i]) /*
Re: [PATCH] drm/msm/dpu: remove extra drm_encoder_cleanup from the error path
On 12/11/2023 6:54 AM, Dmitry Baryshkov wrote: The drmm handler will perform drm_encoder_cleanup() for us. Moreover if we call drm_encoder_cleanup() manually, the drmm_encoder_alloc_release() will spawn warnings at drivers/gpu/drm/drm_encoder.c:214. Drop these extra drm_encoder_cleanup() calls. Fixes: cd42c56d9c0b ("drm/msm/dpu: use drmm-managed allocation for dpu_encoder_virt") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 --- 1 file changed, 3 deletions(-) Reviewed-by: Abhinav Kumar Tested-by: Abhinav Kumar #sm8250 CI
Re: [PATCH] drm/msm/dp: call dp_display_get_next_bridge() during probe
On 11/6/2023 4:43 PM, Dmitry Baryshkov wrote: The funcion dp_display_get_next_bridge() can return -EPROBE_DEFER if the next bridge is not (yet) available. However returning -EPROBE_DEFER from msm_dp_modeset_init() is not ideal. This leads to -EPROBE return from component_bind, which can easily result in -EPROBE_DEFR loops. Signed-off-by: Dmitry Baryshkov --- Dependencies: https://patchwork.freedesktop.org/series/120375/ --- Reviewed-by: Kuogee Hsieh drivers/gpu/drm/msm/dp/dp_display.c | 36 + 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index d542db37763a..ddb3c84f39a2 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1197,15 +1197,27 @@ static const struct msm_dp_desc *dp_display_get_desc(struct platform_device *pde return NULL; } -static int dp_auxbus_done_probe(struct drm_dp_aux *aux) +static int dp_display_get_next_bridge(struct msm_dp *dp); + +static int dp_display_probe_tail(struct device *dev) { - int rc; + struct msm_dp *dp = dev_get_drvdata(dev); + int ret; - rc = component_add(aux->dev, _display_comp_ops); - if (rc) - DRM_ERROR("eDP component add failed, rc=%d\n", rc); + ret = dp_display_get_next_bridge(dp); + if (ret) + return ret; - return rc; + ret = component_add(dev, _display_comp_ops); + if (ret) + DRM_ERROR("component add failed, rc=%d\n", ret); + + return ret; +} + +static int dp_auxbus_done_probe(struct drm_dp_aux *aux) +{ + return dp_display_probe_tail(aux->dev); } static int dp_display_probe(struct platform_device *pdev) @@ -1280,11 +1292,9 @@ static int dp_display_probe(struct platform_device *pdev) goto err; } } else { - rc = component_add(>dev, _display_comp_ops); - if (rc) { - DRM_ERROR("component add failed, rc=%d\n", rc); + rc = dp_display_probe_tail(>dev); + if (rc) goto err; - } } return rc; @@ -1415,7 +1425,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp) * For DisplayPort interfaces external bridges are optional, so * silently ignore an error if one is not present (-ENODEV). */ - rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser); + rc = devm_dp_parser_find_next_bridge(>pdev->dev, dp_priv->parser); if (!dp->is_edp && rc == -ENODEV) return 0; @@ -1435,10 +1445,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev, dp_priv = container_of(dp_display, struct dp_display_private, dp_display); - ret = dp_display_get_next_bridge(dp_display); - if (ret) - return ret; - ret = dp_bridge_init(dp_display, dev, encoder); if (ret) { DRM_DEV_ERROR(dev->dev,
Re: [PATCH v2 05/16] drm/msm/dpu: add cdm blocks to sc7280 dpu_hw_catalog
On 12/8/2023 3:19 AM, Dmitry Baryshkov wrote: On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar wrote: Add CDM blocks to the sc7280 dpu_hw_catalog to support YUV format output from writeback block. changes in v2: - remove explicit zero assignment for features - move sc7280_cdm to dpu_hw_catalog from the sc7280 catalog file as its definition can be re-used Signed-off-by: Abhinav Kumar --- .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 5 + 4 files changed, 29 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h index 209675de6742..19c2b7454796 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h @@ -248,6 +248,7 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = { .mdss_ver = _mdss_ver, .caps = _dpu_caps, .mdp = _mdp, + .cdm = _cdm, .ctl_count = ARRAY_SIZE(sc7280_ctl), .ctl = sc7280_ctl, .sspp_count = ARRAY_SIZE(sc7280_sspp), 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 d52aae54bbd5..1be3156cde05 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -426,6 +426,16 @@ static const struct dpu_dsc_sub_blks dsc_sblk_1 = { .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10}, }; +/* + * CDM sub block config Nit: it is not a subblock config. Ack. + */ +static const struct dpu_cdm_cfg sc7280_cdm = { I know that I have r-b'ed this patch. But then one thing occurred to me. If this definition is common to all (or almost all) platforms, can we just call it dpu_cdm or dpu_common_cdm? + .name = "cdm_0", + .id = CDM_0, + .len = 0x228, + .base = 0x79200, +}; hmmm almost common but not entirely ... msm8998's CDM has a shorter len of 0x224 :( + /* * VBIF sub blocks config */ 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 e3c0d007481b..ba82ef4560a6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -682,6 +682,17 @@ struct dpu_vbif_cfg { u32 memtype[MAX_XIN_COUNT]; }; +/** + * struct dpu_cdm_cfg - information of chroma down blocks + * @name string name for debug purposes + * @id enum identifying this block + * @base register offset of this block + * @features bit mask identifying sub-blocks/features + */ +struct dpu_cdm_cfg { + DPU_HW_BLK_INFO; +}; + /** * Define CDP use cases * @DPU_PERF_CDP_UDAGE_RT: real-time use cases @@ -805,6 +816,8 @@ struct dpu_mdss_cfg { u32 wb_count; const struct dpu_wb_cfg *wb; + const struct dpu_cdm_cfg *cdm; + u32 ad_count; u32 dspp_count; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h index a6702b2bfc68..f319c8232ea5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h @@ -185,6 +185,11 @@ enum dpu_dsc { DSC_MAX }; +enum dpu_cdm { + CDM_0 = 1, + CDM_MAX +}; + enum dpu_pingpong { PINGPONG_NONE, PINGPONG_0, -- 2.40.1
Re: [PATCH 1/9] dt-bindings: display: msm: dp: declare compatible string for sm8150
On 10/12/2023 00:21, Dmitry Baryshkov wrote: > Add compatible string for the DisplayPort controller found on the > Qualcomm SM8150 platform. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [Freedreno] [PATCH v2] drm/msm/dpu: improve DSC allocation
On Mon, 11 Dec 2023 at 19:59, Kuogee Hsieh wrote: > > > On 12/6/2023 1:35 AM, Dmitry Baryshkov wrote: > > On 05/12/2023 22:51, Kuogee Hsieh wrote: > >> > >> On 12/5/2023 11:23 AM, Dmitry Baryshkov wrote: > >>> On Tue, 5 Dec 2023 at 20:12, Kuogee Hsieh > >>> wrote: > > On 12/4/2023 4:23 PM, Dmitry Baryshkov wrote: > > On Tue, 5 Dec 2023 at 01:55, Kuogee Hsieh > > wrote: > >> A DCE (Display Compression Engine) contains two DSC hard slice > >> encoders. Each DCE start with even DSC encoder index followed by > >> an odd DSC encoder index. Each encoder can work independently. > >> But Only two DSC encoders from same DCE can be paired to work > >> together to support merge mode. In addition, the DSC with even > >> index have to mapping to even pingpong index and DSC with odd > >> index have to mapping to odd pingpong index at its data path. > >> This patch improve DSC allocation mechanism with consideration > >> of above factors. > >> > >> Changes in V2: > >> -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and > >> _dpu_rm_reserve_dsc_pair() > > Please don't send the new iteration of the patch if the discussion > > is ongoing. > Got it, thanks. > > Quoting v1 review: > > > > Are the limitations (odd:odd, allocation in pairs, etc) applicable to > > v1.1 encoders? > > > > I assume that at least 'allocate two consecutive DSC for DSC > > merge' is > > not applicable, since there are no separate DCE units. > yes, you are correct in the hardware point of view. > > However, at software, we can think think of dsc index 0 and 1 are > bound > to DCE #1, index 2 and 3 are bound to DCE #2 and etc in regardless of > v1.1 or v1.2. > > By doing that,this dsc allocation algorithm should be able to apply to > both. > > There is no case to have dsc index 1 and dsc index 2 bind together > (skip > dsc index 0) to support merge mode. > >>> Yes. However this might cause issues on the platforms which have DSI, > >>> DP and just two DSC encoders. Consider RM allocating two odd (or two > >>> even) PP blocks. One for DSI, one for DP. Then if we need DSC on both > >>> interfaces, the RM won't be able to allocate it. > >> > >> > >> I am not sure this case is possible. > >> > >> DSC + pingpong allocation is base on Layer mixer which is allocated > >> sequentially. > > > > Not sequentially, but also in pairs. Yes, LM_5 (a pair for LM_2 on > > sdm845) is connected to PINGPONG_3. However all this doesn't make > > things easier to understand or to snoop bugs. I'd prefer to keep a > > simple allocation code for older DSC blocks. Especially since I might > > have to touch LM allocation for MDP 1.x platforms. > > > >> ex, first lm --> pingpong --> dsc allocate completed then followed by > >> next lm --> pingpong --> dsc allocation. > >> > >> therefore it is not possible to have case with two odd pingpong index > >> to map two odd dsc index. > >> > >> With this algorithm, there is one case (below) which can not be > >> supported is, > >> > >> dsc_0 for pingpong-0 of stand alone mode + dsc-1 and dsc-2 for > >> pingpong-1 and ping pong-2 to support merge mode for DSC v1.1. > >> > >> However there is no hardware configuration which only have 3 or 5 > >> dsc encoders due to dsc always come in pair except some low end chip > >> which mdp come with only one dsc encoder. > >> > >> > >> > >> > >>> > >> Signed-off-by: Kuogee Hsieh > >> --- > >>drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 171 > >> ++--- > >>1 file changed, 156 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > >> index 17ecf23..dafe1ee 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > >> @@ -470,33 +470,174 @@ static int _dpu_rm_reserve_ctls( > >> return 0; > >>} > >> > >> -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm, > >> +static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm, > >> struct dpu_global_state > >> *global_state, > >> - struct drm_encoder *enc, > >> + uint32_t enc_id, > >> const struct > >> msm_display_topology *top) > >>{ > >> - int num_dsc = top->num_dsc; > >> - int i; > >> + int num_dsc = 0; > >> + int i, pp_idx; > >> + int dsc_idx[DSC_MAX - DSC_0]; > >> + uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0]; > > > > You don't need pp_to_enc_id copy in the _single usecase. Just use the > > exisiting rm data. > > > >> + int pp_max = PINGPONG_MAX - PINGPONG_0; > > > > inline > > > >> +
[PATCH v3] drm/msm/dpu: improve DSC allocation
A DCE (Display Compression Engine) contains two DSC hard slice encoders. Each DCE start with even DSC encoder index followed by an odd DSC encoder index. Each encoder can work independently. But Only two DSC encoders from same DCE can be paired to work together to support merge mode. In addition, the DSC with even index have to mapping to even pingpong index and DSC with odd index have to mapping to odd pingpong index at its data path. This patch improve DSC allocation mechanism with consideration of above factors. Changes in V3: -- add dpu_rm_pingpong_dsc_check() -- for pair allocation use i += 2 at for loop Changes in V2: -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and _dpu_rm_reserve_dsc_pair() Fixes: f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in RM") Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 171 ++--- 1 file changed, 155 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 17ecf23..43598ee 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -470,33 +470,172 @@ static int _dpu_rm_reserve_ctls( return 0; } -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm, +static int _dpu_rm_pingpong_dsc_check(int dsc_idx, + uint32_t enc_id, + uint32_t *pp_to_enc_id, + int pp_max, + bool pair) +{ + int pp_idx; + + /* +* find the pingpong index which had been reserved +* previously at layer mixer allocation +*/ + for (pp_idx = 0; pp_idx < pp_max; pp_idx++) { + if (pp_to_enc_id[pp_idx] == enc_id) + break; + } + + /* +* dsc even index must map to pingpong even index +* dsc odd index must map to pingpong odd index +*/ + if ((dsc_idx & 0x01) != (pp_idx & 0x01)) + return -ENAVAIL; + + if (pair) { + /* +* delete pp_idx so that same pp_id can not be paired with +* next dsc_id +*/ + pp_to_enc_id[pp_idx] = 0x; + } + + return 0; + +} + +static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm, struct dpu_global_state *global_state, - struct drm_encoder *enc, + uint32_t enc_id, const struct msm_display_topology *top) { - int num_dsc = top->num_dsc; - int i; + int num_dsc = 0; + int i, ret; + int dsc_id[DSC_MAX - DSC_0]; + uint32_t *pp_enc_id = global_state->pingpong_to_enc_id; + int pp_max = PINGPONG_MAX - PINGPONG_0; - /* check if DSC required are allocated or not */ - for (i = 0; i < num_dsc; i++) { - if (!rm->dsc_blks[i]) { - DPU_ERROR("DSC %d does not exist\n", i); - return -EIO; - } + memset(dsc_id, 0, sizeof(dsc_id)); - if (global_state->dsc_to_enc_id[i]) { - DPU_ERROR("DSC %d is already allocated\n", i); - return -EIO; - } + for (i = 0; i < ARRAY_SIZE(rm->dsc_blks) && num_dsc >= top->num_dsc; i++) { + if (!rm->dsc_blks[i]) + continue; + + if (global_state->dsc_to_enc_id[i]) /* used */ + continue; + + ret = _dpu_rm_pingpong_dsc_check(i, enc_id, pp_enc_id, pp_max, false); + if (!ret) + dsc_id[num_dsc++] = DSC_0 + i; /* found, start from DSC_0 */ } - for (i = 0; i < num_dsc; i++) - global_state->dsc_to_enc_id[i] = enc->base.id; + if (num_dsc < top->num_dsc) { + DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n", + num_dsc, top->num_dsc); + return -ENAVAIL; + } + + /* allocate dsc */ + for (i = 0; i < top->num_dsc; i++) { + int id; + + id = dsc_id[i]; + if (id >= DSC_0) + global_state->dsc_to_enc_id[id - DSC_0] = enc_id; + } return 0; } +static int _dpu_rm_reserve_dsc_pair(struct dpu_rm *rm, + struct dpu_global_state *global_state, + uint32_t enc_id, + const struct msm_display_topology *top) +{ + int num_dsc = 0; + int i, ret; + int dsc_id[DSC_MAX - DSC_0]; + uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0]; + uint32_t *dsc_enc_id = global_state->dsc_to_enc_id; + int pp_max = PINGPONG_MAX - PINGPONG_0; + + memset(dsc_id, 0,
Re: [PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
On 12/11/2023 10:19 AM, Rob Clark wrote: From: Rob Clark When we start getting these, we get a *lot*. So ratelimit it to not flood dmesg. Signed-off-by: Rob Clark --- dpu should probably stop rolling it's own trace macros, but that would be a larger cleanup. drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) Reviewed-by: Abhinav Kumar
[PATCH] drm/msm/dpu: Ratelimit framedone timeout msgs
From: Rob Clark When we start getting these, we get a *lot*. So ratelimit it to not flood dmesg. Signed-off-by: Rob Clark --- dpu should probably stop rolling it's own trace macros, but that would be a larger cleanup. drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 82538844614b..7c22235d0eba 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -39,6 +39,9 @@ #define DPU_ERROR_ENC(e, fmt, ...) DPU_ERROR("enc%d " fmt,\ (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) +#define DPU_ERROR_ENC_RATELIMITED(e, fmt, ...) DPU_ERROR_RATELIMITED("enc%d " fmt,\ + (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) + /* * Two to anticipate panels that can do cmd/vid dynamic switching * plan is to create all possible physical encoder types, and switch between @@ -2339,7 +2342,7 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t) return; } - DPU_ERROR_ENC(dpu_enc, "frame done timeout\n"); + DPU_ERROR_ENC_RATELIMITED(dpu_enc, "frame done timeout\n"); event = DPU_ENCODER_FRAME_EVENT_ERROR; trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index b6f53ca6e962..f5473d4dea92 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -51,6 +51,7 @@ } while (0) #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__) +#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt, ##__VA_ARGS__) /** * ktime_compare_safe - compare two ktime structures -- 2.43.0
Re: [PATCH] drm/msm/dp: call dp_display_get_next_bridge() during probe
On Tue, Nov 07, 2023 at 02:43:33AM +0200, Dmitry Baryshkov wrote: > The funcion dp_display_get_next_bridge() can return -EPROBE_DEFER if the > next bridge is not (yet) available. However returning -EPROBE_DEFER from > msm_dp_modeset_init() is not ideal. This leads to -EPROBE return from > component_bind, which can easily result in -EPROBE_DEFR loops. > Nice! > Signed-off-by: Dmitry Baryshkov > --- > > Dependencies: https://patchwork.freedesktop.org/series/120375/ > > --- > drivers/gpu/drm/msm/dp/dp_display.c | 36 + > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index d542db37763a..ddb3c84f39a2 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -1197,15 +1197,27 @@ static const struct msm_dp_desc > *dp_display_get_desc(struct platform_device *pde > return NULL; > } > > -static int dp_auxbus_done_probe(struct drm_dp_aux *aux) > +static int dp_display_get_next_bridge(struct msm_dp *dp); > + > +static int dp_display_probe_tail(struct device *dev) > { > - int rc; > + struct msm_dp *dp = dev_get_drvdata(dev); > + int ret; > > - rc = component_add(aux->dev, _display_comp_ops); > - if (rc) > - DRM_ERROR("eDP component add failed, rc=%d\n", rc); > + ret = dp_display_get_next_bridge(dp); > + if (ret) > + return ret; > > - return rc; > + ret = component_add(dev, _display_comp_ops); > + if (ret) > + DRM_ERROR("component add failed, rc=%d\n", ret); > + > + return ret; > +} > + > +static int dp_auxbus_done_probe(struct drm_dp_aux *aux) > +{ > + return dp_display_probe_tail(aux->dev); > } > > static int dp_display_probe(struct platform_device *pdev) > @@ -1280,11 +1292,9 @@ static int dp_display_probe(struct platform_device > *pdev) > goto err; > } > } else { > - rc = component_add(>dev, _display_comp_ops); > - if (rc) { > - DRM_ERROR("component add failed, rc=%d\n", rc); > + rc = dp_display_probe_tail(>dev); > + if (rc) > goto err; > - } > } > > return rc; > @@ -1415,7 +1425,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp) >* For DisplayPort interfaces external bridges are optional, so >* silently ignore an error if one is not present (-ENODEV). >*/ > - rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser); > + rc = devm_dp_parser_find_next_bridge(>pdev->dev, dp_priv->parser); This transition worried me, but after reading the code the current model of mixing devices for devres scares me more. So, nice cleanup! But I think we have a few more of these... That said, >pdev->dev is dp_priv->parser->dev, the function no longer relate to the "parser module", and we stash the return value of devm_drm_of_get_bridge(dev, dev->of_node, 1, 0) in parser->next_brigde, so that we 5 lines below this call can move it into dp->next_bridge. As such, I'd like to propose that we change devm_dp_parser_find_next_bridge() to just take >pdev->dev and return the next_bridge, in an ERR_PTR(). But that's follow-up-patch material. Reviewed-by: Bjorn Andersson Regards, Bjorn > if (!dp->is_edp && rc == -ENODEV) > return 0; > > @@ -1435,10 +1445,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, > struct drm_device *dev, > > dp_priv = container_of(dp_display, struct dp_display_private, > dp_display); > > - ret = dp_display_get_next_bridge(dp_display); > - if (ret) > - return ret; > - > ret = dp_bridge_init(dp_display, dev, encoder); > if (ret) { > DRM_DEV_ERROR(dev->dev, > -- > 2.42.0 > >
Re: [Freedreno] [PATCH v2] drm/msm/dpu: improve DSC allocation
On 12/6/2023 1:35 AM, Dmitry Baryshkov wrote: On 05/12/2023 22:51, Kuogee Hsieh wrote: On 12/5/2023 11:23 AM, Dmitry Baryshkov wrote: On Tue, 5 Dec 2023 at 20:12, Kuogee Hsieh wrote: On 12/4/2023 4:23 PM, Dmitry Baryshkov wrote: On Tue, 5 Dec 2023 at 01:55, Kuogee Hsieh wrote: A DCE (Display Compression Engine) contains two DSC hard slice encoders. Each DCE start with even DSC encoder index followed by an odd DSC encoder index. Each encoder can work independently. But Only two DSC encoders from same DCE can be paired to work together to support merge mode. In addition, the DSC with even index have to mapping to even pingpong index and DSC with odd index have to mapping to odd pingpong index at its data path. This patch improve DSC allocation mechanism with consideration of above factors. Changes in V2: -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and _dpu_rm_reserve_dsc_pair() Please don't send the new iteration of the patch if the discussion is ongoing. Got it, thanks. Quoting v1 review: Are the limitations (odd:odd, allocation in pairs, etc) applicable to v1.1 encoders? I assume that at least 'allocate two consecutive DSC for DSC merge' is not applicable, since there are no separate DCE units. yes, you are correct in the hardware point of view. However, at software, we can think think of dsc index 0 and 1 are bound to DCE #1, index 2 and 3 are bound to DCE #2 and etc in regardless of v1.1 or v1.2. By doing that,this dsc allocation algorithm should be able to apply to both. There is no case to have dsc index 1 and dsc index 2 bind together (skip dsc index 0) to support merge mode. Yes. However this might cause issues on the platforms which have DSI, DP and just two DSC encoders. Consider RM allocating two odd (or two even) PP blocks. One for DSI, one for DP. Then if we need DSC on both interfaces, the RM won't be able to allocate it. I am not sure this case is possible. DSC + pingpong allocation is base on Layer mixer which is allocated sequentially. Not sequentially, but also in pairs. Yes, LM_5 (a pair for LM_2 on sdm845) is connected to PINGPONG_3. However all this doesn't make things easier to understand or to snoop bugs. I'd prefer to keep a simple allocation code for older DSC blocks. Especially since I might have to touch LM allocation for MDP 1.x platforms. ex, first lm --> pingpong --> dsc allocate completed then followed by next lm --> pingpong --> dsc allocation. therefore it is not possible to have case with two odd pingpong index to map two odd dsc index. With this algorithm, there is one case (below) which can not be supported is, dsc_0 for pingpong-0 of stand alone mode + dsc-1 and dsc-2 for pingpong-1 and ping pong-2 to support merge mode for DSC v1.1. However there is no hardware configuration which only have 3 or 5 dsc encoders due to dsc always come in pair except some low end chip which mdp come with only one dsc encoder. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 171 ++--- 1 file changed, 156 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 17ecf23..dafe1ee 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -470,33 +470,174 @@ static int _dpu_rm_reserve_ctls( return 0; } -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm, +static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm, struct dpu_global_state *global_state, - struct drm_encoder *enc, + uint32_t enc_id, const struct msm_display_topology *top) { - int num_dsc = top->num_dsc; - int i; + int num_dsc = 0; + int i, pp_idx; + int dsc_idx[DSC_MAX - DSC_0]; + uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0]; You don't need pp_to_enc_id copy in the _single usecase. Just use the exisiting rm data. + int pp_max = PINGPONG_MAX - PINGPONG_0; inline + + for (i = 0; i < DSC_MAX - DSC_0; i++) + dsc_idx[i] = 0; You know, this is called memset. + + /* fill working copy with pingpong list */ + memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id, sizeof(pp_to_enc_id)); + + for (i = 0; i < ARRAY_SIZE(rm->dsc_blks) && num_dsc >= top->num_dsc; i++) { + if (!rm->dsc_blks[i]) + continue; - /* check if DSC required are allocated or not */ - for (i = 0; i < num_dsc; i++) { - if (!rm->dsc_blks[i]) { - DPU_ERROR("DSC %d does not exist\n", i); - return -EIO; + if (global_state->dsc_to_enc_id[i]) /* used */ + continue; + + /* + * find the pingpong index
Re: drm/msm/dp: call dp_display_get_next_bridge() during probe
On 7.11.2023 01:43, Dmitry Baryshkov wrote: > The funcion dp_display_get_next_bridge() can return -EPROBE_DEFER if the > next bridge is not (yet) available. However returning -EPROBE_DEFER from > msm_dp_modeset_init() is not ideal. This leads to -EPROBE return from > component_bind, which can easily result in -EPROBE_DEFR loops. > > Signed-off-by: Dmitry Baryshkov > --- Tested-by: Konrad Dybcio # sc8180x-primus Konrad
Re: [PATCH] drm/msm/dpu: remove extra drm_encoder_cleanup from the error path
On Mon, 11 Dec 2023 at 16:54, Dmitry Baryshkov wrote: > > The drmm handler will perform drm_encoder_cleanup() for us. Moreover if > we call drm_encoder_cleanup() manually, the drmm_encoder_alloc_release() > will spawn warnings at drivers/gpu/drm/drm_encoder.c:214. Drop these > extra drm_encoder_cleanup() calls. > > Fixes: cd42c56d9c0b ("drm/msm/dpu: use drmm-managed allocation for > dpu_encoder_virt") > Signed-off-by: Dmitry Baryshkov Reported-by: Konrad Dybcio -- With best wishes Dmitry
[PATCH v2 8/8] arm64: dts: qcom: sm8150-hdk: enable DisplayPort and USB-C altmode
Enable the USB-C related functionality for the USB-C port on this board. This includes OTG, PowerDelivery and DP AltMode. Also enable the DisplayPort itself. Signed-off-by: Dmitry Baryshkov --- arch/arm64/boot/dts/qcom/sm8150-hdk.dts | 124 +++- 1 file changed, 123 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sm8150-hdk.dts b/arch/arm64/boot/dts/qcom/sm8150-hdk.dts index ea4d75308ac8..de670b407ef1 100644 --- a/arch/arm64/boot/dts/qcom/sm8150-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8150-hdk.dts @@ -7,6 +7,7 @@ #include #include +#include #include "sm8150.dtsi" #include "pm8150.dtsi" #include "pm8150b.dtsi" @@ -374,6 +375,10 @@ { status = "okay"; }; +_dma0 { + status = "okay"; +}; + _dma1 { status = "okay"; }; @@ -382,6 +387,29 @@ { status = "okay"; }; + { + clock-frequency = <10>; + + status = "okay"; + + typec-mux@42 { + compatible = "fcs,fsa4480"; + reg = <0x42>; + + interrupts-extended = < 152 IRQ_TYPE_LEVEL_LOW>; + + vcc-supply = <_bob>; + mode-switch; + orientation-switch; + + port { + fsa4480_sbu_mux: endpoint { + remote-endpoint = <_typec_sbu_out>; + }; + }; + }; +}; + { status = "okay"; clock-frequency = <40>; @@ -436,6 +464,15 @@ { status = "okay"; }; +_dp { + status = "okay"; +}; + +_dp_out { + data-lanes = <0 1>; + remote-endpoint = <_1_qmpphy_dp_in>; +}; + _dsi0 { status = "okay"; vdda-supply = <_l3c_1p2>; @@ -483,6 +520,65 @@ _dsi1_phy { status = "okay"; }; +_vbus { + regulator-min-microamp = <50>; + regulator-max-microamp = <300>; + status = "okay"; +}; + +_typec { + status = "okay"; + + vdd-pdphy-supply = <_l2a_3p1>; + + connector { + compatible = "usb-c-connector"; + + power-role = "source"; + data-role = "dual"; + self-powered; + + source-pdos = ; + + altmodes { + displayport { + svid = /bits/ 16 <0xff01>; + vdo = <0x1c46>; + }; + }; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + pm8150b_role_switch_in: endpoint { + remote-endpoint = <_1_dwc3_hs>; + }; + }; + + port@1 { + reg = <1>; + pm8150b_typec_mux_in: endpoint { + remote-endpoint = <_1_qmpphy_out>; + }; + }; + + port@2 { + reg = <2>; + + pm8150b_typec_sbu_out: endpoint { + remote-endpoint = <_sbu_mux>; + }; + }; + }; + }; +}; + _pwrkey { status = "okay"; }; @@ -493,6 +589,10 @@ _resin { linux,code = ; }; +_id_0 { + status = "okay"; +}; + _id_1 { status = "okay"; }; @@ -568,6 +668,19 @@ _1_qmpphy { status = "okay"; vdda-phy-supply = <_l3c_1p2>; vdda-pll-supply = <_l18a_0p8>; + orientation-switch; +}; + +_1_qmpphy_dp_in { + remote-endpoint = <_dp_out>; +}; + +_1_qmpphy_out { + remote-endpoint = <_typec_mux_in>; +}; + +_1_qmpphy_usb_ss_in { + remote-endpoint = <_1_dwc3_ss>; }; _2_qmpphy { @@ -585,7 +698,16 @@ _2 { }; _1_dwc3 { - dr_mode = "peripheral"; + dr_mode = "otg"; + usb-role-switch; +}; + +_1_dwc3_hs { + remote-endpoint = <_role_switch_in>; +}; + +_1_dwc3_ss { + remote-endpoint = <_1_qmpphy_usb_ss_in>; }; _2_dwc3 { -- 2.39.2
[PATCH v2 7/8] arm64: dts: qcom: sm8150: add USB-C ports to the OTG USB host
Expand first USB host controller device node with the OF ports required to support USB-C / DisplayPort switching. Signed-off-by: Dmitry Baryshkov --- arch/arm64/boot/dts/qcom/sm8150.dtsi | 19 +++ 1 file changed, 19 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi index 77d32f4fe7da..168d49b01807 100644 --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi @@ -3608,6 +3608,25 @@ usb_1_dwc3: usb@a60 { snps,dis_enblslpm_quirk; phys = <_1_hsphy>, <_1_qmpphy QMP_USB43DP_USB3_PHY>; phy-names = "usb2-phy", "usb3-phy"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + usb_1_dwc3_hs: endpoint { + }; + }; + + port@1 { + reg = <1>; + + usb_1_dwc3_ss: endpoint { + }; + }; + }; }; }; -- 2.39.2
[PATCH v2 6/8] arm64: dts: qcom: sm8150: add USB-C ports to the USB+DP QMP PHY
Expand Combo USB+DP QMP PHY device node with the OF ports required to support USB-C / DisplayPort switching. Signed-off-by: Dmitry Baryshkov --- arch/arm64/boot/dts/qcom/sm8150.dtsi | 26 ++ 1 file changed, 26 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi index ea7c92c0e405..77d32f4fe7da 100644 --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi @@ -3447,6 +3447,32 @@ usb_1_qmpphy: phy@88e8000 { #phy-cells = <1>; status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + usb_1_qmpphy_out: endpoint { + }; + }; + + port@1 { + reg = <1>; + + usb_1_qmpphy_usb_ss_in: endpoint { + }; + }; + + port@2 { + reg = <2>; + + usb_1_qmpphy_dp_in: endpoint { + }; + }; + }; }; usb_2_qmpphy: phy@88eb000 { -- 2.39.2
[PATCH v2 4/8] arm64: dts: qcom: sm8150-hdk: fix SS USB regulators
The SM8150-HDK uses two different regulators to power up SuperSpeed USB PHYs. The L5A regulator is used for the second USB host, while the first (OTG) USB host uses different regulator, L18A. Fix the regulator for the usb_1 QMPPHY and (to remove possible confusion) drop the usb_ss_dp_core_1/_2 labels. Fixes: 0ab1b2d10afe ("arm64: dts: qcom: add sm8150 hdk dts") Reviewed-by: Konrad Dybcio Signed-off-by: Dmitry Baryshkov --- arch/arm64/boot/dts/qcom/sm8150-hdk.dts | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sm8150-hdk.dts b/arch/arm64/boot/dts/qcom/sm8150-hdk.dts index 6a036f9ba1c9..ea4d75308ac8 100644 --- a/arch/arm64/boot/dts/qcom/sm8150-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8150-hdk.dts @@ -138,8 +138,6 @@ vdda_qrefs_0p875_5: vdda_sp_sensor: vdda_ufs_2ln_core_1: vdda_ufs_2ln_core_2: - vdda_usb_ss_dp_core_1: - vdda_usb_ss_dp_core_2: vdda_qlink_lv: vdda_qlink_lv_ck: vreg_l5a_0p875: ldo5 { @@ -221,6 +219,12 @@ vreg_l17a_3p0: ldo17 { regulator-max-microvolt = <3008000>; regulator-initial-mode = ; }; + + vreg_l18a_0p8: ldo18 { + regulator-min-microvolt = <88>; + regulator-max-microvolt = <88>; + regulator-initial-mode = ; + }; }; regulators-1 { @@ -563,13 +567,13 @@ _2_hsphy { _1_qmpphy { status = "okay"; vdda-phy-supply = <_l3c_1p2>; - vdda-pll-supply = <_usb_ss_dp_core_1>; + vdda-pll-supply = <_l18a_0p8>; }; _2_qmpphy { status = "okay"; vdda-phy-supply = <_l3c_1p2>; - vdda-pll-supply = <_usb_ss_dp_core_1>; + vdda-pll-supply = <_l5a_0p875>; }; _1 { -- 2.39.2
[PATCH v2 2/8] arm64: dts: qcom: sm8150: make dispcc cast minimal vote on MMCX
Add required-opps property to the display clock controller. This makes it cast minimal vote on the MMCX lane and prevents further 'clock stuck' errors when enabling the display. Fixes: 2ef3bb17c45c ("arm64: dts: qcom: sm8150: Add DISPCC node") Acked-by: Konrad Dybcio Signed-off-by: Dmitry Baryshkov --- arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi index fb41f91cefc6..153c531c1d41 100644 --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi @@ -3925,6 +3925,7 @@ dispcc: clock-controller@af0 { "dp_phy_pll_link_clk", "dp_phy_pll_vco_div_clk"; power-domains = < SM8150_MMCX>; + required-opps = <_opp_low_svs>; #clock-cells = <1>; #reset-cells = <1>; #power-domain-cells = <1>; -- 2.39.2
[PATCH v2 3/8] arm64: dts: qcom: sm8150-hdk: enable HDMI output
Add DSI outputs and link them to the onboard Lontium LT9611 DSI-to-HDMI bridge, enabling HDMI output on this board. While adding the display resources, also drop the headless ("amd,imageon") compat string from the GPU node, since the board now has output. Signed-off-by: Dmitry Baryshkov --- arch/arm64/boot/dts/qcom/sm8150-hdk.dts | 128 +++- 1 file changed, 123 insertions(+), 5 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sm8150-hdk.dts b/arch/arm64/boot/dts/qcom/sm8150-hdk.dts index bb161b536da4..6a036f9ba1c9 100644 --- a/arch/arm64/boot/dts/qcom/sm8150-hdk.dts +++ b/arch/arm64/boot/dts/qcom/sm8150-hdk.dts @@ -54,6 +54,17 @@ key-vol-up { gpios = <_gpios 6 GPIO_ACTIVE_LOW>; }; }; + + hdmi-out { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con: endpoint { + remote-endpoint = <_out>; + }; + }; + }; }; _rsc { @@ -359,12 +370,112 @@ { status = "okay"; }; +_dma1 { + status = "okay"; +}; + { - /* -* NOTE: "amd,imageon" makes Adreno start in headless mode, remove it -* after display support is added on this board. -*/ - compatible = "qcom,adreno-640.1", "qcom,adreno", "amd,imageon"; + status = "okay"; +}; + + { + status = "okay"; + clock-frequency = <40>; + + lt9611_codec: hdmi-bridge@3b { + compatible = "lontium,lt9611"; + reg = <0x3b>; + #sound-dai-cells = <1>; + + interrupts-extended = < 9 IRQ_TYPE_EDGE_FALLING>; + + reset-gpios = < 7 GPIO_ACTIVE_HIGH>; + + vdd-supply = <_s4a_1p8>; + vcc-supply = <_bob>; + + pinctrl-names = "default"; + pinctrl-0 = <_irq_pin>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + lt9611_a: endpoint { + remote-endpoint = <_dsi0_out>; + }; + }; + + port@1 { + reg = <1>; + + lt9611_b: endpoint { + remote-endpoint = <_dsi1_out>; + }; + }; + + port@2 { + reg = <2>; + + lt9611_out: endpoint { + remote-endpoint = <_con>; + }; + }; + }; + }; +}; + + { + status = "okay"; +}; + +_dsi0 { + status = "okay"; + vdda-supply = <_l3c_1p2>; + + qcom,dual-dsi-mode; + qcom,master-dsi; + + ports { + port@1 { + endpoint { + remote-endpoint = <_a>; + data-lanes = <0 1 2 3>; + }; + }; + }; +}; + +_dsi0_phy { + status = "okay"; + vdds-supply = <_l5a_0p875>; +}; + +_dsi1 { + vdda-supply = <_l3c_1p2>; + + qcom,dual-dsi-mode; + + /* DSI1 is slave, so use DSI0 clocks */ + assigned-clock-parents = <_dsi0_phy 0>, <_dsi0_phy 1>; + + status = "okay"; + + ports { + port@1 { + endpoint { + remote-endpoint = <_b>; + data-lanes = <0 1 2 3>; + }; + }; + }; +}; + +_dsi1_phy { + vdds-supply = <_l5a_0p875>; status = "okay"; }; @@ -402,6 +513,13 @@ _slpi { { gpio-reserved-ranges = <0 4>, <126 4>; + + lt9611_irq_pin: lt9611-irq-state { + pins = "gpio9"; + function = "gpio"; + bias-disable; + }; + }; { -- 2.39.2
[PATCH v2 5/8] arm64: dts: qcom: sm8150: add DisplayPort controller
Add device tree node for the DisplayPort controller and link it to the display controller interface. Signed-off-by: Dmitry Baryshkov --- arch/arm64/boot/dts/qcom/sm8150.dtsi | 87 1 file changed, 87 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi index 153c531c1d41..ea7c92c0e405 100644 --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi @@ -3712,6 +3712,13 @@ dpu_intf2_out: endpoint { remote-endpoint = <_dsi1_in>; }; }; + + port@2 { + reg = <2>; + dpu_intf0_out: endpoint { + remote-endpoint = <_dp_in>; + }; + }; }; mdp_opp_table: opp-table { @@ -3739,6 +3746,86 @@ opp-46000 { }; }; + mdss_dp: displayport-controller@ae9 { + compatible = "qcom,sm8150-dp", "qcom,sm8350-dp"; + reg = <0 0xae9 0 0x200>, + <0 0xae90200 0 0x200>, + <0 0xae90400 0 0x600>, + <0 0x0ae90a00 0 0x600>, + <0 0x0ae91000 0 0x600>; + + interrupt-parent = <>; + interrupts = <12>; + clocks = < DISP_CC_MDSS_AHB_CLK>, +< DISP_CC_MDSS_DP_AUX_CLK>, +< DISP_CC_MDSS_DP_LINK_CLK>, +< DISP_CC_MDSS_DP_LINK_INTF_CLK>, +< DISP_CC_MDSS_DP_PIXEL_CLK>; + clock-names = "core_iface", + "core_aux", + "ctrl_link", + "ctrl_link_iface", + "stream_pixel"; + + assigned-clocks = < DISP_CC_MDSS_DP_LINK_CLK_SRC>, + < DISP_CC_MDSS_DP_PIXEL_CLK_SRC>; + assigned-clock-parents = <_1_qmpphy QMP_USB43DP_DP_LINK_CLK>, +<_1_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>; + + phys = <_1_qmpphy QMP_USB43DP_DP_PHY>; + phy-names = "dp"; + + #sound-dai-cells = <0>; + + operating-points-v2 = <_opp_table>; + power-domains = < SM8250_MMCX>; + + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + mdss_dp_in: endpoint { + remote-endpoint = <_intf0_out>; + }; + }; + + port@1 { + reg = <1>; + + mdss_dp_out: endpoint { + }; + }; + }; + + dp_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-16000 { + opp-hz = /bits/ 64 <16000>; + required-opps = <_opp_low_svs>; + }; + + opp-27000 { + opp-hz = /bits/ 64 <27000>; + required-opps = <_opp_svs>; + }; + + opp-54000 { + opp-hz = /bits/ 64 <54000>; + required-opps = <_opp_svs_l1>; + }; + +
[PATCH v2 1/8] dt-bindings: display: msm: dp: declare compatible string for sm8150
Add compatible string for the DisplayPort controller found on the Qualcomm SM8150 platform. Signed-off-by: Dmitry Baryshkov --- Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml index dbe398f84ffb..f850bd9b8263 100644 --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml @@ -28,6 +28,7 @@ properties: - qcom,sm8350-dp - items: - enum: + - qcom,sm8150-dp - qcom,sm8250-dp - qcom,sm8450-dp - qcom,sm8550-dp -- 2.39.2
[PATCH v2 0/8] arm64: dts: qcom: sm8150-hdk: enable display output
Enable display output on the SM8150 HDK device. This includes HDMI output through the onboard DSI-HDMI bridge and DP output on the USB-C port. Changes since v1 - Dropped irrelevant stats patch - Fixed endpoint stye (Konrad) - Changed SVID from u32 to 16-bits value (Konrad) Dmitry Baryshkov (8): dt-bindings: display: msm: dp: declare compatible string for sm8150 arm64: dts: qcom: sm8150: make dispcc cast minimal vote on MMCX arm64: dts: qcom: sm8150-hdk: enable HDMI output arm64: dts: qcom: sm8150-hdk: fix SS USB regulators arm64: dts: qcom: sm8150: add DisplayPort controller arm64: dts: qcom: sm8150: add USB-C ports to the USB+DP QMP PHY arm64: dts: qcom: sm8150: add USB-C ports to the OTG USB host arm64: dts: qcom: sm8150-hdk: enable DisplayPort and USB-C altmode .../bindings/display/msm/dp-controller.yaml | 1 + arch/arm64/boot/dts/qcom/sm8150-hdk.dts | 264 +- arch/arm64/boot/dts/qcom/sm8150.dtsi | 133 + 3 files changed, 388 insertions(+), 10 deletions(-) -- 2.39.2
[PATCH] drm/msm/dpu: remove extra drm_encoder_cleanup from the error path
The drmm handler will perform drm_encoder_cleanup() for us. Moreover if we call drm_encoder_cleanup() manually, the drmm_encoder_alloc_release() will spawn warnings at drivers/gpu/drm/drm_encoder.c:214. Drop these extra drm_encoder_cleanup() calls. Fixes: cd42c56d9c0b ("drm/msm/dpu: use drmm-managed allocation for dpu_encoder_virt") Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index dc24fe4bb3b0..d60edb93d4f7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -586,7 +586,6 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev, rc = msm_dp_modeset_init(priv->dp[i], dev, encoder); if (rc) { DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc); - drm_encoder_cleanup(encoder); return rc; } } @@ -619,7 +618,6 @@ static int _dpu_kms_initialize_hdmi(struct drm_device *dev, rc = msm_hdmi_modeset_init(priv->hdmi, dev, encoder); if (rc) { DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc); - drm_encoder_cleanup(encoder); return rc; } @@ -651,7 +649,6 @@ static int _dpu_kms_initialize_writeback(struct drm_device *dev, n_formats); if (rc) { DPU_ERROR("dpu_writeback_init, rc = %d\n", rc); - drm_encoder_cleanup(encoder); return rc; } -- 2.39.2
Re: [PATCH 1/3] iommu/msm-iommu: don't limit the driver too much
On Thu, Dec 07, 2023 at 01:06:09PM +, Robin Murphy wrote: > On 07/12/2023 12:54 pm, Dmitry Baryshkov wrote: > > In preparation of dropping most of ARCH_QCOM subtypes, stop limiting the > > driver just to those machines. Allow it to be built for any 32-bit > > Qualcomm platform (ARCH_QCOM). > > Acked-by: Robin Murphy > > Unless Joerg disagrees, I think it should be fine if you want to take this > via the SoC tree. No objections: Acked-by: Joerg Roedel
Re: [Linaro-mm-sig] [PATCH] drm/scheduler: Unwrap job dependencies
Am 05.12.23 um 20:02 schrieb Rob Clark: From: Rob Clark Container fences have burner contexts, which makes the trick to store at most one fence per context somewhat useless if we don't unwrap array or chain fences. Signed-off-by: Rob Clark Reviewed-by: Christian König --- drivers/gpu/drm/scheduler/sched_main.c | 47 ++ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 9762464e3f99..16b550949c57 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -52,6 +52,7 @@ #include #include #include +#include #include #include @@ -684,27 +685,14 @@ void drm_sched_job_arm(struct drm_sched_job *job) } EXPORT_SYMBOL(drm_sched_job_arm); -/** - * drm_sched_job_add_dependency - adds the fence as a job dependency - * @job: scheduler job to add the dependencies to - * @fence: the dma_fence to add to the list of dependencies. - * - * Note that @fence is consumed in both the success and error cases. - * - * Returns: - * 0 on success, or an error on failing to expand the array. - */ -int drm_sched_job_add_dependency(struct drm_sched_job *job, -struct dma_fence *fence) +static int drm_sched_job_add_single_dependency(struct drm_sched_job *job, + struct dma_fence *fence) { struct dma_fence *entry; unsigned long index; u32 id = 0; int ret; - if (!fence) - return 0; - /* Deduplicate if we already depend on a fence from the same context. * This lets the size of the array of deps scale with the number of * engines involved, rather than the number of BOs. @@ -728,6 +716,35 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job, return ret; } + +/** + * drm_sched_job_add_dependency - adds the fence as a job dependency + * @job: scheduler job to add the dependencies to + * @fence: the dma_fence to add to the list of dependencies. + * + * Note that @fence is consumed in both the success and error cases. + * + * Returns: + * 0 on success, or an error on failing to expand the array. + */ +int drm_sched_job_add_dependency(struct drm_sched_job *job, +struct dma_fence *fence) +{ + struct dma_fence_unwrap iter; + struct dma_fence *f; + int ret = 0; + + dma_fence_unwrap_for_each (f, , fence) { + dma_fence_get(f); + ret = drm_sched_job_add_single_dependency(job, f); + if (ret) + break; + } + + dma_fence_put(fence); + + return ret; +} EXPORT_SYMBOL(drm_sched_job_add_dependency); /**
Re: [PATCH v2] iommu/arm-smmu-qcom: Add missing GMU entry to match table
On Sun, Dec 10, 2023 at 10:06:53AM -0800, Rob Clark wrote: > From: Rob Clark > > In some cases the firmware expects cbndx 1 to be assigned to the GMU, > so we also want the default domain for the GMU to be an identy domain. > This way it does not get a context bank assigned. Without this, both > of_dma_configure() and drm/msm's iommu_domain_attach() will trigger > allocating and configuring a context bank. So GMU ends up attached to > both cbndx 1 and later cbndx 2. This arrangement seemingly confounds > and surprises the firmware if the GPU later triggers a translation > fault, resulting (on sc8280xp / lenovo x13s, at least) in the SMMU > getting wedged and the GPU stuck without memory access. > > Cc: sta...@vger.kernel.org > Signed-off-by: Rob Clark Tested-by: Johan Hovold
Re: [PATCH 7/9] arm64: dts: qcom: sm8150: add USB-C ports to the USB+DP QMP PHY
On 11.12.2023 10:46, Dmitry Baryshkov wrote: > On Mon, 11 Dec 2023 at 11:33, Konrad Dybcio wrote: >> >> On 10.12.2023 00:21, Dmitry Baryshkov wrote: >>> Expand Combo USB+DP QMP PHY device node with the OF ports required to >>> support USB-C / DisplayPort switching. >>> >>> Signed-off-by: Dmitry Baryshkov >>> --- >>> arch/arm64/boot/dts/qcom/sm8150.dtsi | 23 +++ >>> 1 file changed, 23 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi >>> b/arch/arm64/boot/dts/qcom/sm8150.dtsi >>> index ea7c92c0e405..38423a9f8408 100644 >>> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi >>> @@ -3447,6 +3447,29 @@ usb_1_qmpphy: phy@88e8000 { >>> #phy-cells = <1>; >>> >>> status = "disabled"; >>> + >>> + ports { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + port@0 { >>> + reg = <0>; >>> + usb_1_qmpphy_out: endpoint {}; >> style 1 >>> + }; >>> + >>> + port@1 { >>> + reg = <1>; >>> + >>> + usb_1_qmpphy_usb_ss_in: endpoint { >>> + }; >> style 2 >>> + }; >>> + >>> + port@2 { >>> + reg = <2>; >>> + >>> + usb_1_qmpphy_dp_in: endpoint {}; >> style 3 >> >> :( > > Which one should I stick to? style 2 seems to be used in 8650 Konrad
Re: [PATCH 7/9] arm64: dts: qcom: sm8150: add USB-C ports to the USB+DP QMP PHY
On Mon, 11 Dec 2023 at 11:33, Konrad Dybcio wrote: > > On 10.12.2023 00:21, Dmitry Baryshkov wrote: > > Expand Combo USB+DP QMP PHY device node with the OF ports required to > > support USB-C / DisplayPort switching. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > arch/arm64/boot/dts/qcom/sm8150.dtsi | 23 +++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi > > b/arch/arm64/boot/dts/qcom/sm8150.dtsi > > index ea7c92c0e405..38423a9f8408 100644 > > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > > @@ -3447,6 +3447,29 @@ usb_1_qmpphy: phy@88e8000 { > > #phy-cells = <1>; > > > > status = "disabled"; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + usb_1_qmpphy_out: endpoint {}; > style 1 > > + }; > > + > > + port@1 { > > + reg = <1>; > > + > > + usb_1_qmpphy_usb_ss_in: endpoint { > > + }; > style 2 > > + }; > > + > > + port@2 { > > + reg = <2>; > > + > > + usb_1_qmpphy_dp_in: endpoint {}; > style 3 > > :( Which one should I stick to? -- With best wishes Dmitry
Re: [PATCH 9/9] arm64: dts: qcom: sm8150-hdk: enable DisplayPort and USB-C altmode
On Mon, 11 Dec 2023 at 11:33, Konrad Dybcio wrote: > > On 10.12.2023 00:21, Dmitry Baryshkov wrote: > > Enable the USB-C related functionality for the USB-C port on this board. > > This includes OTG, PowerDelivery and DP AltMode. Also enable the > > DisplayPort itself. > > > > Signed-off-by: Dmitry Baryshkov > > --- > [...] > > > +_typec { > > + status = "okay"; > > + > > + vdd-pdphy-supply = <_l2a_3p1>; > > + > > + connector { > > + compatible = "usb-c-connector"; > > + > > + power-role = "source"; > > + data-role = "dual"; > > + self-powered; > > + > > + source-pdos = > + PDO_FIXED_DUAL_ROLE | > > + PDO_FIXED_USB_COMM | > > + PDO_FIXED_DATA_SWAP)>; > > + > > + altmodes { > > + displayport { > > + svid = <0xff01>; > /bits/ 16? Ugh, yes. -- With best wishes Dmitry
Re: [PATCH 4/9] arm64: dts: qcom: sm8150-hdk: enable HDMI output
On Mon, 11 Dec 2023 at 11:31, Konrad Dybcio wrote: > > On 10.12.2023 00:21, Dmitry Baryshkov wrote: > > Add DSI outputs and link them to the onboard Lontium LT9611 DSI-to-HDMI > > bridge, enabling HDMI output on this board. While adding the display > > resources, also drop the headless ("amd,imageon") compat string from the > > GPU node, since the board now has output. > > > > Signed-off-by: Dmitry Baryshkov > > --- > [...] > > > > + > > + lt9611_irq_pin: lt9611-irq-state { > > + pins = "gpio9"; > > + function = "gpio"; > > + bias-disable; > No drive-strength? For the input pin with no bias? I'm not sure. And yes, it was c from RB3. > Otherwise lokos good at a glance! -- With best wishes Dmitry
Re: [PATCH 9/9] arm64: dts: qcom: sm8150-hdk: enable DisplayPort and USB-C altmode
On 10.12.2023 00:21, Dmitry Baryshkov wrote: > Enable the USB-C related functionality for the USB-C port on this board. > This includes OTG, PowerDelivery and DP AltMode. Also enable the > DisplayPort itself. > > Signed-off-by: Dmitry Baryshkov > --- [...] > +_typec { > + status = "okay"; > + > + vdd-pdphy-supply = <_l2a_3p1>; > + > + connector { > + compatible = "usb-c-connector"; > + > + power-role = "source"; > + data-role = "dual"; > + self-powered; > + > + source-pdos = + PDO_FIXED_DUAL_ROLE | > + PDO_FIXED_USB_COMM | > + PDO_FIXED_DATA_SWAP)>; > + > + altmodes { > + displayport { > + svid = <0xff01>; /bits/ 16? Konrad
Re: [PATCH 7/9] arm64: dts: qcom: sm8150: add USB-C ports to the USB+DP QMP PHY
On 10.12.2023 00:21, Dmitry Baryshkov wrote: > Expand Combo USB+DP QMP PHY device node with the OF ports required to > support USB-C / DisplayPort switching. > > Signed-off-by: Dmitry Baryshkov > --- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi > b/arch/arm64/boot/dts/qcom/sm8150.dtsi > index ea7c92c0e405..38423a9f8408 100644 > --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi > @@ -3447,6 +3447,29 @@ usb_1_qmpphy: phy@88e8000 { > #phy-cells = <1>; > > status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + usb_1_qmpphy_out: endpoint {}; style 1 > + }; > + > + port@1 { > + reg = <1>; > + > + usb_1_qmpphy_usb_ss_in: endpoint { > + }; style 2 > + }; > + > + port@2 { > + reg = <2>; > + > + usb_1_qmpphy_dp_in: endpoint {}; style 3 :( Konrad
Re: [PATCH 5/9] arm64: dts: qcom: sm8150-hdk: fix SS USB regulators
On 10.12.2023 00:21, Dmitry Baryshkov wrote: > The SM8150-HDK uses two different regulators to power up SuperSpeed USB > PHYs. The L5A regulator is used for the second USB host, while the first > (OTG) USB host uses different regulator, L18A. Fix the regulator for the > usb_1 QMPPHY and (to remove possible confusion) drop the > usb_ss_dp_core_1/_2 labels. > > Fixes: 0ab1b2d10afe ("arm64: dts: qcom: add sm8150 hdk dts") > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH 4/9] arm64: dts: qcom: sm8150-hdk: enable HDMI output
On 10.12.2023 00:21, Dmitry Baryshkov wrote: > Add DSI outputs and link them to the onboard Lontium LT9611 DSI-to-HDMI > bridge, enabling HDMI output on this board. While adding the display > resources, also drop the headless ("amd,imageon") compat string from the > GPU node, since the board now has output. > > Signed-off-by: Dmitry Baryshkov > --- [...] > + > + lt9611_irq_pin: lt9611-irq-state { > + pins = "gpio9"; > + function = "gpio"; > + bias-disable; No drive-strength? Otherwise lokos good at a glance! Konrad
Re: [PATCH 3/9] arm64: dts: qcom: sm8150: make dispcc cast minimal vote on MMCX
On 10.12.2023 00:21, Dmitry Baryshkov wrote: > Add required-opps property to the display clock controller. This makes > it cast minimal vote on the MMCX lane and prevents further 'clock stuck' > errors when enabling the display. > > Fixes: 2ef3bb17c45c ("arm64: dts: qcom: sm8150: Add DISPCC node") > Signed-off-by: Dmitry Baryshkov > --- Acked-by: Konrad Dybcio Konrad
Re: [PATCH 2/9] arm64: dts: qcom: sm8150: use SoC-specific compat for RPMh stats
On 10.12.2023 00:21, Dmitry Baryshkov wrote: > The SM8150 platform doesn't support DDR sleep stats, so it needs > SoC-specific compat string for the RPMh stats data. > > Signed-off-by: Dmitry Baryshkov > --- Not a fan, see my comments over at [1]. Konrad [1] https://lore.kernel.org/linux-arm-msm/20231209215601.3543895-1-dmitry.barysh...@linaro.org/T/#t