[Freedreno] [PATCH] drm/msm/dpu: Add UBWC support for RGB8888 formats

2019-11-06 Thread Fritz Koenig
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

2019-11-06 Thread kholk11
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

2019-11-06 Thread Sean Paul
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

2019-11-06 Thread Jordan Crouse
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

2019-11-06 Thread Rob Clark
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

2019-11-06 Thread Jeffrey Hugo
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

2019-11-06 Thread Rob Clark
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

2019-11-06 Thread Rob Clark
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

2019-11-06 Thread Sean Paul
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

2019-11-06 Thread Sharat Masetty
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

2019-11-06 Thread Brian Masney
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