[Freedreno] [PATCH] drm/msm/dpu: Add UBWC support for RGB8888 formats
Hardware only natively supports BGR UBWC. UBWC support for RGB can be had by pretending that the buffer is BGR. Signed-off-by: Fritz Koenig --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 18 ++ .../drm/msm/disp/dpu1/dpu_hw_catalog_format.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 24ab6249083a..528632690f1e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -489,12 +489,28 @@ static const struct dpu_format dpu_format_map_ubwc[] = { true, 4, DPU_FORMAT_FLAG_COMPRESSED, DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC), + /* ARGB and ABGR purposely have the same color +* ordering. The hardware only supports ABGR UBWC +* natively. +*/ + INTERLEAVED_RGB_FMT_TILED(ARGB, + COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, + C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, + true, 4, DPU_FORMAT_FLAG_COMPRESSED, + DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC), + INTERLEAVED_RGB_FMT_TILED(XBGR, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, false, 4, DPU_FORMAT_FLAG_COMPRESSED, DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC), + INTERLEAVED_RGB_FMT_TILED(XRGB, + COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, + C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, + false, 4, DPU_FORMAT_FLAG_COMPRESSED, + DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_UBWC), + INTERLEAVED_RGB_FMT_TILED(ABGR2101010, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, @@ -550,7 +566,9 @@ static int _dpu_format_get_media_color_ubwc(const struct dpu_format *fmt) { static const struct dpu_media_color_map dpu_media_ubwc_map[] = { {DRM_FORMAT_ABGR, COLOR_FMT_RGBA_UBWC}, + {DRM_FORMAT_ARGB, COLOR_FMT_RGBA_UBWC}, {DRM_FORMAT_XBGR, COLOR_FMT_RGBA_UBWC}, + {DRM_FORMAT_XRGB, COLOR_FMT_RGBA_UBWC}, {DRM_FORMAT_ABGR2101010, COLOR_FMT_RGBA1010102_UBWC}, {DRM_FORMAT_XBGR2101010, COLOR_FMT_RGBA1010102_UBWC}, {DRM_FORMAT_BGR565, COLOR_FMT_RGB565_UBWC}, diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h index bb6112c949ae..fbcb3c4bbfee 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h @@ -6,7 +6,9 @@ static const uint32_t qcom_compressed_supported_formats[] = { DRM_FORMAT_ABGR, + DRM_FORMAT_ARGB, DRM_FORMAT_XBGR, + DRM_FORMAT_XRGB, DRM_FORMAT_BGR565, }; -- 2.24.0.432.g9d3f5f5b63-goog ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [RESEND, v4, 2/7] dt-bindings: msm/mdp5: Document optional TBU and TBU_RT clocks
From: AngeloGioacchino Del Regno These two clocks aren't present in all versions of the MDP5 HW: where present, they are needed to enable the Translation Buffer Unit(s). Signed-off-by: AngeloGioacchino Del Regno Acked-by: Rob Herring --- Documentation/devicetree/bindings/display/msm/mdp5.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/mdp5.txt b/Documentation/devicetree/bindings/display/msm/mdp5.txt index 4e11338548aa..43d11279c925 100644 --- a/Documentation/devicetree/bindings/display/msm/mdp5.txt +++ b/Documentation/devicetree/bindings/display/msm/mdp5.txt @@ -76,6 +76,8 @@ Required properties: Optional properties: - clock-names: the following clocks are optional: * "lut" + * "tbu" + * "tbu_rt" Example: -- 2.21.0 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm: msm: a6xx: fix debug bus register configuration
On Wed, Nov 06, 2019 at 05:19:23PM +0530, Sharat Masetty wrote: > Fix the cx debugbus related register configuration, to collect accurate > bus data during gpu snapshot. This helps with complete snapshot dump > and also complete proper GPU recovery. > > Signed-off-by: Sharat Masetty Applied to drm-misc-next-fixes Thanks, Sean > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 24 > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > index 483e100..c5764b4 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > @@ -353,26 +353,26 @@ static void a6xx_get_debugbus(struct msm_gpu *gpu, > cxdbg = ioremap(res->start, resource_size(res)); > > if (cxdbg) { > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLT, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLT, > A6XX_DBGC_CFG_DBGBUS_CNTLT_SEGT(0xf)); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLM, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLM, > A6XX_DBGC_CFG_DBGBUS_CNTLM_ENABLE(0xf)); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_0, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_1, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_2, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_3, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_0, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_1, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_2, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_3, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_0, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_0, > 0x76543210); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_1, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_1, > 0xFEDCBA98); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_0, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_1, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_2, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_3, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_0, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_1, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_2, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_3, 0); > } > > nr_debugbus_blocks = ARRAY_SIZE(a6xx_debugbus_blocks) + > -- > 1.9.1 > > ___ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm: msm: a6xx: fix debug bus register configuration
On Wed, Nov 06, 2019 at 08:18:59AM -0800, Rob Clark wrote: > On Wed, Nov 6, 2019 at 3:49 AM Sharat Masetty wrote: > > > > Fix the cx debugbus related register configuration, to collect accurate > > bus data during gpu snapshot. This helps with complete snapshot dump > > and also complete proper GPU recovery. > > > > Signed-off-by: Sharat Masetty This commit summary is far too polite. The word boneheaded should have appeared several times. Thanks for this patch. > (adding fixes tag for benefit of stable kernels) > > Fixes: 1707add81551 ("drm/msm/a6xx: Add a6xx gpu state") Thanks, I was going to suggest this as well. > Reviewed-by: Rob Clark Reviewed-by: Jordan Crouse > > --- > > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 24 > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > > b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > > index 483e100..c5764b4 100644 > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > > @@ -353,26 +353,26 @@ static void a6xx_get_debugbus(struct msm_gpu *gpu, > > cxdbg = ioremap(res->start, resource_size(res)); > > > > if (cxdbg) { > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLT, > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLT, > > A6XX_DBGC_CFG_DBGBUS_CNTLT_SEGT(0xf)); > > > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLM, > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLM, > > A6XX_DBGC_CFG_DBGBUS_CNTLM_ENABLE(0xf)); > > > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_0, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_1, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_2, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_3, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_0, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_1, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_2, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_3, 0); > > > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_0, > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_0, > > 0x76543210); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_1, > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_1, > > 0xFEDCBA98); > > > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_0, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_1, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_2, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_3, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_0, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_1, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_2, 0); > > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_3, 0); > > } > > > > nr_debugbus_blocks = ARRAY_SIZE(a6xx_debugbus_blocks) + > > -- > > 1.9.1 > > > > ___ > > Freedreno mailing list > > Freedreno@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/freedreno > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] drm/msm: 'pp done time out' errors after async commit changes
On Wed, Nov 6, 2019 at 8:47 AM Jeffrey Hugo wrote: > > On Wed, Nov 6, 2019 at 9:30 AM Rob Clark wrote: > > > > On Wed, Nov 6, 2019 at 1:13 AM Brian Masney wrote: > > > > > > On Tue, Nov 05, 2019 at 08:23:27AM -0800, Rob Clark wrote: > > > > On Tue, Nov 5, 2019 at 2:08 AM Brian Masney > > > > wrote: > > > > > The 'pp done time out' errors go away if I revert the following three > > > > > commits: > > > > > > > > > > cd6d923167b1 ("drm/msm/dpu: async commit support") > > > > > d934a712c5e6 ("drm/msm: add atomic traces") > > > > > 2d99ced787e3 ("drm/msm: async commit support") > > > > > > > > > > I reverted the first one to fix a compiler error, and the second one > > > > > so > > > > > that the last patch can be reverted without any merge conflicts. > > > > > > > > > > I see that crtc_flush() calls mdp5_ctl_commit(). I tried to use > > > > > crtc_flush_all() in mdp5_flush_commit() and the contents of the frame > > > > > buffer dance around the screen like its out of sync. I renamed > > > > > crtc_flush_all() to mdp5_crtc_flush_all() and removed the static > > > > > declaration. Here's the relevant part of what I tried: > > > > > > > > > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > > > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > > > > @@ -171,7 +171,15 @@ static void mdp5_prepare_commit(struct msm_kms > > > > > *kms, struct drm_atomic_state *st > > > > > > > > > > static void mdp5_flush_commit(struct msm_kms *kms, unsigned > > > > > crtc_mask) > > > > > { > > > > > - /* TODO */ > > > > > + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); > > > > > + struct drm_crtc *crtc; > > > > > + > > > > > + for_each_crtc_mask(mdp5_kms->dev, crtc, crtc_mask) { > > > > > + if (!crtc->state->active) > > > > > + continue; > > > > > + > > > > > + mdp5_crtc_flush_all(crtc); > > > > > + } > > > > > } > > > > > > > > > > Any tips would be appreciated. > > > > > > > > > > > > I think this is along the lines of what we need to enable async commit > > > > for mdp5 (but also removing the flush from the atomic-commit path).. > > > > the principle behind the async commit is to do all the atomic state > > > > commit normally, but defer writing the flush bits. This way, if you > > > > get another async update before the next vblank, you just apply it > > > > immediately instead of waiting for vblank. > > > > > > > > But I guess you are on a command mode panel, if I remember? Which is > > > > a case I didn't have a way to test. And I'm not entirely about how > > > > kms_funcs->vsync_time() should be implemented for cmd mode panels. > > > > > > Yes, this is a command-mode panel and there's no hardware frame counter > > > available. The key to getting the display working on this phone was this > > > patch: > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2bab52af6fe68c43b327a57e5ce5fc10eefdfadf > > > > > > > That all said, I think we should first fix what is broken, before > > > > worrying about extending async commit support to mdp5.. which > > > > shouldn't hit the async==true path, due to not implementing > > > > kms_funcs->vsync_time(). > > > > > > > > What I think is going on is that, in the cmd mode case, > > > > mdp5_wait_flush() (indirectly) calls mdp5_crtc_wait_for_pp_done(), > > > > which waits for a pp-done irq regardless of whether there is a flush > > > > in progress. Since there is no flush pending, the irq never comes. > > > > But the expectation is that kms_funcs->wait_flush() returns > > > > immediately if there is nothing to wait for. > > > > > > I don't think that's happening in this case. I added some pr_info() > > > statements to request_pp_done_pending() and mdp5_crtc_pp_done_irq(). > > > Here's the first two sets of messages that appear in dmesg: > > > > > > [ 14.018907] msm fd90.mdss: pp done time out, lm=0 > > > [ 14.018993] request_pp_done_pending: HERE > > > [ 14.074208] mdp5_crtc_pp_done_irq: HERE > > > [ 14.074368] Console: switching to colour frame buffer device 135x120 > > > [ 14.138938] msm fd90.mdss: pp done time out, lm=0 > > > [ 14.139021] request_pp_done_pending: HERE > > > [ 14.158097] mdp5_crtc_pp_done_irq: HERE > > > > > > The messages go on like this with the same pattern. > > > > > > I tried two different changes: > > > > > > 1) I moved the request_pp_done_pending() and corresponding if statement > > >from mdp5_crtc_atomic_flush() and into mdp5_crtc_atomic_begin(). > > > > > > 2) I increased the timeout in wait_for_completion_timeout() by several > > >increments; all the way to 5 seconds. > > > > increasing the timeout won't help, because the pp-done irq has already > > come at the point where we wait for it.. > > > > maybe the easy thing is just add mdp5_crtc->needs_pp, set to true > > before requesting, and false when we get the irq.. and then > > mdp5_crtc_wait_for_pp_done() just returns if needs_pp==false.. > >
Re: [Freedreno] drm/msm: 'pp done time out' errors after async commit changes
On Wed, Nov 6, 2019 at 9:30 AM Rob Clark wrote: > > On Wed, Nov 6, 2019 at 1:13 AM Brian Masney wrote: > > > > On Tue, Nov 05, 2019 at 08:23:27AM -0800, Rob Clark wrote: > > > On Tue, Nov 5, 2019 at 2:08 AM Brian Masney wrote: > > > > The 'pp done time out' errors go away if I revert the following three > > > > commits: > > > > > > > > cd6d923167b1 ("drm/msm/dpu: async commit support") > > > > d934a712c5e6 ("drm/msm: add atomic traces") > > > > 2d99ced787e3 ("drm/msm: async commit support") > > > > > > > > I reverted the first one to fix a compiler error, and the second one so > > > > that the last patch can be reverted without any merge conflicts. > > > > > > > > I see that crtc_flush() calls mdp5_ctl_commit(). I tried to use > > > > crtc_flush_all() in mdp5_flush_commit() and the contents of the frame > > > > buffer dance around the screen like its out of sync. I renamed > > > > crtc_flush_all() to mdp5_crtc_flush_all() and removed the static > > > > declaration. Here's the relevant part of what I tried: > > > > > > > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > > > @@ -171,7 +171,15 @@ static void mdp5_prepare_commit(struct msm_kms > > > > *kms, struct drm_atomic_state *st > > > > > > > > static void mdp5_flush_commit(struct msm_kms *kms, unsigned crtc_mask) > > > > { > > > > - /* TODO */ > > > > + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); > > > > + struct drm_crtc *crtc; > > > > + > > > > + for_each_crtc_mask(mdp5_kms->dev, crtc, crtc_mask) { > > > > + if (!crtc->state->active) > > > > + continue; > > > > + > > > > + mdp5_crtc_flush_all(crtc); > > > > + } > > > > } > > > > > > > > Any tips would be appreciated. > > > > > > > > > I think this is along the lines of what we need to enable async commit > > > for mdp5 (but also removing the flush from the atomic-commit path).. > > > the principle behind the async commit is to do all the atomic state > > > commit normally, but defer writing the flush bits. This way, if you > > > get another async update before the next vblank, you just apply it > > > immediately instead of waiting for vblank. > > > > > > But I guess you are on a command mode panel, if I remember? Which is > > > a case I didn't have a way to test. And I'm not entirely about how > > > kms_funcs->vsync_time() should be implemented for cmd mode panels. > > > > Yes, this is a command-mode panel and there's no hardware frame counter > > available. The key to getting the display working on this phone was this > > patch: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2bab52af6fe68c43b327a57e5ce5fc10eefdfadf > > > > > That all said, I think we should first fix what is broken, before > > > worrying about extending async commit support to mdp5.. which > > > shouldn't hit the async==true path, due to not implementing > > > kms_funcs->vsync_time(). > > > > > > What I think is going on is that, in the cmd mode case, > > > mdp5_wait_flush() (indirectly) calls mdp5_crtc_wait_for_pp_done(), > > > which waits for a pp-done irq regardless of whether there is a flush > > > in progress. Since there is no flush pending, the irq never comes. > > > But the expectation is that kms_funcs->wait_flush() returns > > > immediately if there is nothing to wait for. > > > > I don't think that's happening in this case. I added some pr_info() > > statements to request_pp_done_pending() and mdp5_crtc_pp_done_irq(). > > Here's the first two sets of messages that appear in dmesg: > > > > [ 14.018907] msm fd90.mdss: pp done time out, lm=0 > > [ 14.018993] request_pp_done_pending: HERE > > [ 14.074208] mdp5_crtc_pp_done_irq: HERE > > [ 14.074368] Console: switching to colour frame buffer device 135x120 > > [ 14.138938] msm fd90.mdss: pp done time out, lm=0 > > [ 14.139021] request_pp_done_pending: HERE > > [ 14.158097] mdp5_crtc_pp_done_irq: HERE > > > > The messages go on like this with the same pattern. > > > > I tried two different changes: > > > > 1) I moved the request_pp_done_pending() and corresponding if statement > >from mdp5_crtc_atomic_flush() and into mdp5_crtc_atomic_begin(). > > > > 2) I increased the timeout in wait_for_completion_timeout() by several > >increments; all the way to 5 seconds. > > increasing the timeout won't help, because the pp-done irq has already > come at the point where we wait for it.. > > maybe the easy thing is just add mdp5_crtc->needs_pp, set to true > before requesting, and false when we get the irq.. and then > mdp5_crtc_wait_for_pp_done() just returns if needs_pp==false.. On the otherhand, what about trying to make command mode panels resemble video mode panels slightly? Video mode panels have a vsync counter in hardware, which is missing from command mode - however it seems like the driver/drm framework would prefer such a counter. Would it be a
Re: [Freedreno] drm/msm: 'pp done time out' errors after async commit changes
On Wed, Nov 6, 2019 at 1:13 AM Brian Masney wrote: > > On Tue, Nov 05, 2019 at 08:23:27AM -0800, Rob Clark wrote: > > On Tue, Nov 5, 2019 at 2:08 AM Brian Masney wrote: > > > The 'pp done time out' errors go away if I revert the following three > > > commits: > > > > > > cd6d923167b1 ("drm/msm/dpu: async commit support") > > > d934a712c5e6 ("drm/msm: add atomic traces") > > > 2d99ced787e3 ("drm/msm: async commit support") > > > > > > I reverted the first one to fix a compiler error, and the second one so > > > that the last patch can be reverted without any merge conflicts. > > > > > > I see that crtc_flush() calls mdp5_ctl_commit(). I tried to use > > > crtc_flush_all() in mdp5_flush_commit() and the contents of the frame > > > buffer dance around the screen like its out of sync. I renamed > > > crtc_flush_all() to mdp5_crtc_flush_all() and removed the static > > > declaration. Here's the relevant part of what I tried: > > > > > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > > @@ -171,7 +171,15 @@ static void mdp5_prepare_commit(struct msm_kms *kms, > > > struct drm_atomic_state *st > > > > > > static void mdp5_flush_commit(struct msm_kms *kms, unsigned crtc_mask) > > > { > > > - /* TODO */ > > > + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); > > > + struct drm_crtc *crtc; > > > + > > > + for_each_crtc_mask(mdp5_kms->dev, crtc, crtc_mask) { > > > + if (!crtc->state->active) > > > + continue; > > > + > > > + mdp5_crtc_flush_all(crtc); > > > + } > > > } > > > > > > Any tips would be appreciated. > > > > > > I think this is along the lines of what we need to enable async commit > > for mdp5 (but also removing the flush from the atomic-commit path).. > > the principle behind the async commit is to do all the atomic state > > commit normally, but defer writing the flush bits. This way, if you > > get another async update before the next vblank, you just apply it > > immediately instead of waiting for vblank. > > > > But I guess you are on a command mode panel, if I remember? Which is > > a case I didn't have a way to test. And I'm not entirely about how > > kms_funcs->vsync_time() should be implemented for cmd mode panels. > > Yes, this is a command-mode panel and there's no hardware frame counter > available. The key to getting the display working on this phone was this > patch: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2bab52af6fe68c43b327a57e5ce5fc10eefdfadf > > > That all said, I think we should first fix what is broken, before > > worrying about extending async commit support to mdp5.. which > > shouldn't hit the async==true path, due to not implementing > > kms_funcs->vsync_time(). > > > > What I think is going on is that, in the cmd mode case, > > mdp5_wait_flush() (indirectly) calls mdp5_crtc_wait_for_pp_done(), > > which waits for a pp-done irq regardless of whether there is a flush > > in progress. Since there is no flush pending, the irq never comes. > > But the expectation is that kms_funcs->wait_flush() returns > > immediately if there is nothing to wait for. > > I don't think that's happening in this case. I added some pr_info() > statements to request_pp_done_pending() and mdp5_crtc_pp_done_irq(). > Here's the first two sets of messages that appear in dmesg: > > [ 14.018907] msm fd90.mdss: pp done time out, lm=0 > [ 14.018993] request_pp_done_pending: HERE > [ 14.074208] mdp5_crtc_pp_done_irq: HERE > [ 14.074368] Console: switching to colour frame buffer device 135x120 > [ 14.138938] msm fd90.mdss: pp done time out, lm=0 > [ 14.139021] request_pp_done_pending: HERE > [ 14.158097] mdp5_crtc_pp_done_irq: HERE > > The messages go on like this with the same pattern. > > I tried two different changes: > > 1) I moved the request_pp_done_pending() and corresponding if statement >from mdp5_crtc_atomic_flush() and into mdp5_crtc_atomic_begin(). > > 2) I increased the timeout in wait_for_completion_timeout() by several >increments; all the way to 5 seconds. increasing the timeout won't help, because the pp-done irq has already come at the point where we wait for it.. maybe the easy thing is just add mdp5_crtc->needs_pp, set to true before requesting, and false when we get the irq.. and then mdp5_crtc_wait_for_pp_done() just returns if needs_pp==false.. BR, -R > I haven't dug into the new code anymore. > > Brian ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] drm: msm: a6xx: fix debug bus register configuration
On Wed, Nov 6, 2019 at 3:49 AM Sharat Masetty wrote: > > Fix the cx debugbus related register configuration, to collect accurate > bus data during gpu snapshot. This helps with complete snapshot dump > and also complete proper GPU recovery. > > Signed-off-by: Sharat Masetty (adding fixes tag for benefit of stable kernels) Fixes: 1707add81551 ("drm/msm/a6xx: Add a6xx gpu state") Reviewed-by: Rob Clark > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 24 > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > index 483e100..c5764b4 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > @@ -353,26 +353,26 @@ static void a6xx_get_debugbus(struct msm_gpu *gpu, > cxdbg = ioremap(res->start, resource_size(res)); > > if (cxdbg) { > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLT, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLT, > A6XX_DBGC_CFG_DBGBUS_CNTLT_SEGT(0xf)); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLM, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLM, > A6XX_DBGC_CFG_DBGBUS_CNTLM_ENABLE(0xf)); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_0, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_1, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_2, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_3, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_0, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_1, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_2, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_3, 0); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_0, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_0, > 0x76543210); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_1, > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_1, > 0xFEDCBA98); > > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_0, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_1, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_2, 0); > - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_3, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_0, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_1, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_2, 0); > + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_3, 0); > } > > nr_debugbus_blocks = ARRAY_SIZE(a6xx_debugbus_blocks) + > -- > 1.9.1 > > ___ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH RESEND 07/14] drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
On Mon, Aug 26, 2019 at 3:27 PM Andrzej Pietrasiewicz wrote: > > Use the ddc pointer provided by the generic connector. > > Signed-off-by: Andrzej Pietrasiewicz > Acked-by: Sam Ravnborg > Reviewed-by: Emil Velikov Acked-by: Sean Paul > --- > drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c > b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c > index 07b4cb877d82..1f03262b8a52 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c > @@ -450,8 +450,10 @@ struct drm_connector *msm_hdmi_connector_init(struct > hdmi *hdmi) > > connector = _connector->base; > > - drm_connector_init(hdmi->dev, connector, _connector_funcs, > - DRM_MODE_CONNECTOR_HDMIA); > + drm_connector_init_with_ddc(hdmi->dev, connector, > + _connector_funcs, > + DRM_MODE_CONNECTOR_HDMIA, > + hdmi->i2c); > drm_connector_helper_add(connector, _hdmi_connector_helper_funcs); > > connector->polled = DRM_CONNECTOR_POLL_CONNECT | > -- > 2.17.1 > ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH] drm: msm: a6xx: fix debug bus register configuration
Fix the cx debugbus related register configuration, to collect accurate bus data during gpu snapshot. This helps with complete snapshot dump and also complete proper GPU recovery. Signed-off-by: Sharat Masetty --- drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c index 483e100..c5764b4 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c @@ -353,26 +353,26 @@ static void a6xx_get_debugbus(struct msm_gpu *gpu, cxdbg = ioremap(res->start, resource_size(res)); if (cxdbg) { - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLT, + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLT, A6XX_DBGC_CFG_DBGBUS_CNTLT_SEGT(0xf)); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_CNTLM, + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_CNTLM, A6XX_DBGC_CFG_DBGBUS_CNTLM_ENABLE(0xf)); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_0, 0); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_1, 0); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_2, 0); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_IVTL_3, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_0, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_1, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_2, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_IVTL_3, 0); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_0, + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_0, 0x76543210); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_BYTEL_1, + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_BYTEL_1, 0xFEDCBA98); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_0, 0); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_1, 0); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_2, 0); - cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_3, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_0, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_1, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_2, 0); + cxdbg_write(cxdbg, REG_A6XX_CX_DBGC_CFG_DBGBUS_MASKL_3, 0); } nr_debugbus_blocks = ARRAY_SIZE(a6xx_debugbus_blocks) + -- 1.9.1 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] drm/msm: 'pp done time out' errors after async commit changes
On Tue, Nov 05, 2019 at 08:23:27AM -0800, Rob Clark wrote: > On Tue, Nov 5, 2019 at 2:08 AM Brian Masney wrote: > > The 'pp done time out' errors go away if I revert the following three > > commits: > > > > cd6d923167b1 ("drm/msm/dpu: async commit support") > > d934a712c5e6 ("drm/msm: add atomic traces") > > 2d99ced787e3 ("drm/msm: async commit support") > > > > I reverted the first one to fix a compiler error, and the second one so > > that the last patch can be reverted without any merge conflicts. > > > > I see that crtc_flush() calls mdp5_ctl_commit(). I tried to use > > crtc_flush_all() in mdp5_flush_commit() and the contents of the frame > > buffer dance around the screen like its out of sync. I renamed > > crtc_flush_all() to mdp5_crtc_flush_all() and removed the static > > declaration. Here's the relevant part of what I tried: > > > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > > @@ -171,7 +171,15 @@ static void mdp5_prepare_commit(struct msm_kms *kms, > > struct drm_atomic_state *st > > > > static void mdp5_flush_commit(struct msm_kms *kms, unsigned crtc_mask) > > { > > - /* TODO */ > > + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); > > + struct drm_crtc *crtc; > > + > > + for_each_crtc_mask(mdp5_kms->dev, crtc, crtc_mask) { > > + if (!crtc->state->active) > > + continue; > > + > > + mdp5_crtc_flush_all(crtc); > > + } > > } > > > > Any tips would be appreciated. > > > I think this is along the lines of what we need to enable async commit > for mdp5 (but also removing the flush from the atomic-commit path).. > the principle behind the async commit is to do all the atomic state > commit normally, but defer writing the flush bits. This way, if you > get another async update before the next vblank, you just apply it > immediately instead of waiting for vblank. > > But I guess you are on a command mode panel, if I remember? Which is > a case I didn't have a way to test. And I'm not entirely about how > kms_funcs->vsync_time() should be implemented for cmd mode panels. Yes, this is a command-mode panel and there's no hardware frame counter available. The key to getting the display working on this phone was this patch: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2bab52af6fe68c43b327a57e5ce5fc10eefdfadf > That all said, I think we should first fix what is broken, before > worrying about extending async commit support to mdp5.. which > shouldn't hit the async==true path, due to not implementing > kms_funcs->vsync_time(). > > What I think is going on is that, in the cmd mode case, > mdp5_wait_flush() (indirectly) calls mdp5_crtc_wait_for_pp_done(), > which waits for a pp-done irq regardless of whether there is a flush > in progress. Since there is no flush pending, the irq never comes. > But the expectation is that kms_funcs->wait_flush() returns > immediately if there is nothing to wait for. I don't think that's happening in this case. I added some pr_info() statements to request_pp_done_pending() and mdp5_crtc_pp_done_irq(). Here's the first two sets of messages that appear in dmesg: [ 14.018907] msm fd90.mdss: pp done time out, lm=0 [ 14.018993] request_pp_done_pending: HERE [ 14.074208] mdp5_crtc_pp_done_irq: HERE [ 14.074368] Console: switching to colour frame buffer device 135x120 [ 14.138938] msm fd90.mdss: pp done time out, lm=0 [ 14.139021] request_pp_done_pending: HERE [ 14.158097] mdp5_crtc_pp_done_irq: HERE The messages go on like this with the same pattern. I tried two different changes: 1) I moved the request_pp_done_pending() and corresponding if statement from mdp5_crtc_atomic_flush() and into mdp5_crtc_atomic_begin(). 2) I increased the timeout in wait_for_completion_timeout() by several increments; all the way to 5 seconds. I haven't dug into the new code anymore. Brian ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno