| Message ID | 20200302203452.17977-34-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel: Fix dotclocks | expand |
Hi, This is a command mode panel and the the msm/mdp5 driver uses the vrefresh field for the actual refresh rate, while the dotclock field is used for the DSI clocks. The dotclock needed to be a bit higher than necessary otherwise the panel would not work. If you want to get rid of the separate clock/vrefresh fields there would need to be some changes to msm driver. (note I hadn't made the patch with upstreaming in mind, the 150000 value is likely not optimal, just something that worked, this is something that should have been checked with the downstream driver) -Jonathan On 3/2/20 3:34 PM, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > The currently listed dotclock disagrees with the currently > listed vrefresh rate. Change the dotclock to match the vrefresh. > > Someone tell me which (if either) of the dotclock or vreresh is > correct? > > Cc: Jonathan Marek <jonathan@marek.ca> > Cc: Brian Masney <masneyb@onstation.org> > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/panel/panel-simple.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index b24fdf239440..f958d8dfd760 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi panasonic_vvx10f004b00 = { > }; > > static const struct drm_display_mode lg_acx467akm_7_mode = { > - .clock = 150000, > + .clock = 125498, > .hdisplay = 1080, > .hsync_start = 1080 + 2, > .hsync_end = 1080 + 2 + 2, >
On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: > Hi, > > This is a command mode panel and the the msm/mdp5 driver uses the vrefresh > field for the actual refresh rate, while the dotclock field is used for the > DSI clocks. The dotclock needed to be a bit higher than necessary otherwise > the panel would not work. > > If you want to get rid of the separate clock/vrefresh fields there would > need to be some changes to msm driver. > > (note I hadn't made the patch with upstreaming in mind, the 150000 value is > likely not optimal, just something that worked, this is something that > should have been checked with the downstream driver) Is this the right clock frequency in the downstream MSM 3.4 kernel that you're talking about? https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 I don't see any obvious clock values in the downstream command mode panel configuration: https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 and everything appears to be working fine. You can add my Tested-by if you end up applying this. Tested-by: Brian Masney <masneyb@onstation.org> Brian > On 3/2/20 3:34 PM, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > The currently listed dotclock disagrees with the currently > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > Someone tell me which (if either) of the dotclock or vreresh is > > correct? > > > > Cc: Jonathan Marek <jonathan@marek.ca> > > Cc: Brian Masney <masneyb@onstation.org> > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > > index b24fdf239440..f958d8dfd760 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi panasonic_vvx10f004b00 = { > > }; > > static const struct drm_display_mode lg_acx467akm_7_mode = { > > - .clock = 150000, > > + .clock = 125498, > > .hdisplay = 1080, > > .hsync_start = 1080 + 2, > > .hsync_end = 1080 + 2 + 2, > >
On 3/2/20 10:13 PM, Brian Masney wrote: > On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: >> Hi, >> >> This is a command mode panel and the the msm/mdp5 driver uses the vrefresh >> field for the actual refresh rate, while the dotclock field is used for the >> DSI clocks. The dotclock needed to be a bit higher than necessary otherwise >> the panel would not work. >> >> If you want to get rid of the separate clock/vrefresh fields there would >> need to be some changes to msm driver. >> >> (note I hadn't made the patch with upstreaming in mind, the 150000 value is >> likely not optimal, just something that worked, this is something that >> should have been checked with the downstream driver) > > Is this the right clock frequency in the downstream MSM 3.4 kernel that > you're talking about? > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 > No, I'm talking about the DSI clock (the driver for it is in drm/msm/dsi/). For a command mode panel the front/back porches aren't relevant, but the dsi pixel/byte clock need to be a bit higher than 1920x1080x60. Since 125498 is a little higher than 124416 that might be enough (there is also rounding of the clock values to consider). > I don't see any obvious clock values in the downstream command mode > panel configuration: > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 > > Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 > and everything appears to be working fine. You can add my Tested-by if > you end up applying this. > > Tested-by: Brian Masney <masneyb@onstation.org> > > Brian > > >> On 3/2/20 3:34 PM, Ville Syrjala wrote: >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> >>> The currently listed dotclock disagrees with the currently >>> listed vrefresh rate. Change the dotclock to match the vrefresh. >>> >>> Someone tell me which (if either) of the dotclock or vreresh is >>> correct? >>> >>> Cc: Jonathan Marek <jonathan@marek.ca> >>> Cc: Brian Masney <masneyb@onstation.org> >>> Cc: Linus Walleij <linus.walleij@linaro.org> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> --- >>> drivers/gpu/drm/panel/panel-simple.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c >>> index b24fdf239440..f958d8dfd760 100644 >>> --- a/drivers/gpu/drm/panel/panel-simple.c >>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>> @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi panasonic_vvx10f004b00 = { >>> }; >>> static const struct drm_display_mode lg_acx467akm_7_mode = { >>> - .clock = 150000, >>> + .clock = 125498, >>> .hdisplay = 1080, >>> .hsync_start = 1080 + 2, >>> .hsync_end = 1080 + 2 + 2, >>>
Another thing: did you verify that the panel still runs at 60hz (and not dropping frames to 30hz)? IIRC that was the behavior with lower clock. On 3/2/20 10:28 PM, Jonathan Marek wrote: > > On 3/2/20 10:13 PM, Brian Masney wrote: >> On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: >>> Hi, >>> >>> This is a command mode panel and the the msm/mdp5 driver uses the >>> vrefresh >>> field for the actual refresh rate, while the dotclock field is used >>> for the >>> DSI clocks. The dotclock needed to be a bit higher than necessary >>> otherwise >>> the panel would not work. >>> >>> If you want to get rid of the separate clock/vrefresh fields there would >>> need to be some changes to msm driver. >>> >>> (note I hadn't made the patch with upstreaming in mind, the 150000 >>> value is >>> likely not optimal, just something that worked, this is something that >>> should have been checked with the downstream driver) >> >> Is this the right clock frequency in the downstream MSM 3.4 kernel that >> you're talking about? >> >> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 >> >> > > No, I'm talking about the DSI clock (the driver for it is in > drm/msm/dsi/). For a command mode panel the front/back porches aren't > relevant, but the dsi pixel/byte clock need to be a bit higher than > 1920x1080x60. Since 125498 is a little higher than 124416 that might be > enough (there is also rounding of the clock values to consider). > >> I don't see any obvious clock values in the downstream command mode >> panel configuration: >> >> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 >> >> >> Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 >> and everything appears to be working fine. You can add my Tested-by if >> you end up applying this. >> >> Tested-by: Brian Masney <masneyb@onstation.org> >> >> Brian >> >> >>> On 3/2/20 3:34 PM, Ville Syrjala wrote: >>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>> >>>> The currently listed dotclock disagrees with the currently >>>> listed vrefresh rate. Change the dotclock to match the vrefresh. >>>> >>>> Someone tell me which (if either) of the dotclock or vreresh is >>>> correct? >>>> >>>> Cc: Jonathan Marek <jonathan@marek.ca> >>>> Cc: Brian Masney <masneyb@onstation.org> >>>> Cc: Linus Walleij <linus.walleij@linaro.org> >>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>> --- >>>> drivers/gpu/drm/panel/panel-simple.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c >>>> b/drivers/gpu/drm/panel/panel-simple.c >>>> index b24fdf239440..f958d8dfd760 100644 >>>> --- a/drivers/gpu/drm/panel/panel-simple.c >>>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>>> @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi >>>> panasonic_vvx10f004b00 = { >>>> }; >>>> static const struct drm_display_mode lg_acx467akm_7_mode = { >>>> - .clock = 150000, >>>> + .clock = 125498, >>>> .hdisplay = 1080, >>>> .hsync_start = 1080 + 2, >>>> .hsync_end = 1080 + 2 + 2, >>>>
On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: > Another thing: did you verify that the panel still runs at 60hz (and not > dropping frames to 30hz)? IIRC that was the behavior with lower clock. Yes, the panel is running at 60 HZ according to the Xorg log with Ville's patch applied: modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 1920 1922 1924 1926 (115.6 kHz eP) I verified there's no underflow errors in dmesg. If I recall correctly, the clock speeds that was in your tree was set too low for the gpu_opp_table (that wouldn't cause this issue), but I seem to recall there were some other clock speed mismatches. The bandwidth requests weren't set on the RPM as well, so maybe that contributed to the problem. That's done upstream with the msm8974 interconnect driver: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c There's a separate known issue with 'pp done time out' errors that occur on the framebuffer that started upstream several months ago with the introduction of async commit support in the MSM driver. I tried working around this by enabling the autorefresh feature but it's not fully working yet and I hit a dead end since there's no docs available publicly for this. The grim details are at: https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ So I'm still OK with Ville's patch going in. Brian > > On 3/2/20 10:28 PM, Jonathan Marek wrote: > > > > On 3/2/20 10:13 PM, Brian Masney wrote: > > > On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: > > > > Hi, > > > > > > > > This is a command mode panel and the the msm/mdp5 driver uses > > > > the vrefresh > > > > field for the actual refresh rate, while the dotclock field is > > > > used for the > > > > DSI clocks. The dotclock needed to be a bit higher than > > > > necessary otherwise > > > > the panel would not work. > > > > > > > > If you want to get rid of the separate clock/vrefresh fields there would > > > > need to be some changes to msm driver. > > > > > > > > (note I hadn't made the patch with upstreaming in mind, the > > > > 150000 value is > > > > likely not optimal, just something that worked, this is something that > > > > should have been checked with the downstream driver) > > > > > > Is this the right clock frequency in the downstream MSM 3.4 kernel that > > > you're talking about? > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 > > > > > > > > > > No, I'm talking about the DSI clock (the driver for it is in > > drm/msm/dsi/). For a command mode panel the front/back porches aren't > > relevant, but the dsi pixel/byte clock need to be a bit higher than > > 1920x1080x60. Since 125498 is a little higher than 124416 that might be > > enough (there is also rounding of the clock values to consider). > > > > > I don't see any obvious clock values in the downstream command mode > > > panel configuration: > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 > > > > > > > > > Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 > > > and everything appears to be working fine. You can add my Tested-by if > > > you end up applying this. > > > > > > Tested-by: Brian Masney <masneyb@onstation.org> > > > > > > Brian > > > > > > > > > > On 3/2/20 3:34 PM, Ville Syrjala wrote: > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > The currently listed dotclock disagrees with the currently > > > > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > > > > > > > Someone tell me which (if either) of the dotclock or vreresh is > > > > > correct? > > > > > > > > > > Cc: Jonathan Marek <jonathan@marek.ca> > > > > > Cc: Brian Masney <masneyb@onstation.org> > > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > --- > > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > > index b24fdf239440..f958d8dfd760 100644 > > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > > @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi > > > > > panasonic_vvx10f004b00 = { > > > > > }; > > > > > static const struct drm_display_mode lg_acx467akm_7_mode = { > > > > > - .clock = 150000, > > > > > + .clock = 125498, > > > > > .hdisplay = 1080, > > > > > .hsync_start = 1080 + 2, > > > > > .hsync_end = 1080 + 2 + 2, > > > > >
What Xorg prints doesn't mean anything. I don't think there will be errors in dmesg, you need to run something that does pageflips as fast as possible and see that the refresh rate is still 60. (modetest with -v, glmark-drm are examples) On 3/3/20 7:26 AM, Brian Masney wrote: > On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: >> Another thing: did you verify that the panel still runs at 60hz (and not >> dropping frames to 30hz)? IIRC that was the behavior with lower clock. > > Yes, the panel is running at 60 HZ according to the Xorg log with > Ville's patch applied: > > modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 > 1920 1922 1924 1926 (115.6 kHz eP) > > I verified there's no underflow errors in dmesg. > > If I recall correctly, the clock speeds that was in your tree was set > too low for the gpu_opp_table (that wouldn't cause this issue), but I > seem to recall there were some other clock speed mismatches. The > bandwidth requests weren't set on the RPM as well, so maybe that > contributed to the problem. That's done upstream with the msm8974 > interconnect driver: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c > > There's a separate known issue with 'pp done time out' errors that > occur on the framebuffer that started upstream several months ago with > the introduction of async commit support in the MSM driver. I tried > working around this by enabling the autorefresh feature but it's not > fully working yet and I hit a dead end since there's no docs available > publicly for this. The grim details are at: > > https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ > > So I'm still OK with Ville's patch going in. > > Brian > > >> >> On 3/2/20 10:28 PM, Jonathan Marek wrote: >>> >>> On 3/2/20 10:13 PM, Brian Masney wrote: >>>> On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: >>>>> Hi, >>>>> >>>>> This is a command mode panel and the the msm/mdp5 driver uses >>>>> the vrefresh >>>>> field for the actual refresh rate, while the dotclock field is >>>>> used for the >>>>> DSI clocks. The dotclock needed to be a bit higher than >>>>> necessary otherwise >>>>> the panel would not work. >>>>> >>>>> If you want to get rid of the separate clock/vrefresh fields there would >>>>> need to be some changes to msm driver. >>>>> >>>>> (note I hadn't made the patch with upstreaming in mind, the >>>>> 150000 value is >>>>> likely not optimal, just something that worked, this is something that >>>>> should have been checked with the downstream driver) >>>> >>>> Is this the right clock frequency in the downstream MSM 3.4 kernel that >>>> you're talking about? >>>> >>>> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 >>>> >>>> >>> >>> No, I'm talking about the DSI clock (the driver for it is in >>> drm/msm/dsi/). For a command mode panel the front/back porches aren't >>> relevant, but the dsi pixel/byte clock need to be a bit higher than >>> 1920x1080x60. Since 125498 is a little higher than 124416 that might be >>> enough (there is also rounding of the clock values to consider). >>> >>>> I don't see any obvious clock values in the downstream command mode >>>> panel configuration: >>>> >>>> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 >>>> >>>> >>>> Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 >>>> and everything appears to be working fine. You can add my Tested-by if >>>> you end up applying this. >>>> >>>> Tested-by: Brian Masney <masneyb@onstation.org> >>>> >>>> Brian >>>> >>>> >>>>> On 3/2/20 3:34 PM, Ville Syrjala wrote: >>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>>> >>>>>> The currently listed dotclock disagrees with the currently >>>>>> listed vrefresh rate. Change the dotclock to match the vrefresh. >>>>>> >>>>>> Someone tell me which (if either) of the dotclock or vreresh is >>>>>> correct? >>>>>> >>>>>> Cc: Jonathan Marek <jonathan@marek.ca> >>>>>> Cc: Brian Masney <masneyb@onstation.org> >>>>>> Cc: Linus Walleij <linus.walleij@linaro.org> >>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/panel/panel-simple.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c >>>>>> b/drivers/gpu/drm/panel/panel-simple.c >>>>>> index b24fdf239440..f958d8dfd760 100644 >>>>>> --- a/drivers/gpu/drm/panel/panel-simple.c >>>>>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>>>>> @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi >>>>>> panasonic_vvx10f004b00 = { >>>>>> }; >>>>>> static const struct drm_display_mode lg_acx467akm_7_mode = { >>>>>> - .clock = 150000, >>>>>> + .clock = 125498, >>>>>> .hdisplay = 1080, >>>>>> .hsync_start = 1080 + 2, >>>>>> .hsync_end = 1080 + 2 + 2, >>>>>>
On Tue, Mar 03, 2020 at 08:04:05AM -0500, Jonathan Marek wrote: > What Xorg prints doesn't mean anything. I don't think there will be errors > in dmesg, you need to run something that does pageflips as fast as possible > and see that the refresh rate is still 60. (modetest with -v, glmark-drm are > examples) I assume that you mean modetest from https://gitlab.freedesktop.org/mesa/drm/tree/master/tests/modetest ? Here's the modeset connector information: id encoder status name size (mm) modes encoders 32 31 connected DSI-1 62x110 1 31 modes: index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) #0 1080x1920 71.71 1080 1082 1084 1086 1920 1922 1924 1926 150000 flags: ; type: preferred, driver And the page flip results... $ modetest -v -s 32:1080x1920 trying to open device 'msm'...done setting mode 1080x1920-71.71Hz@XR24 on connectors 32, crtc 50 failed to set gamma: Function not implemented freq: 13.50Hz freq: 13.51Hz freq: 13.51Hz It's the same results with and without Ville's patch. Here's the beginning of the glmark2 results with the x11-gl flavor: ======================================================= glmark2 2017.07 ======================================================= OpenGL Information GL_VENDOR: freedreno GL_RENDERER: FD330 GL_VERSION: 3.1 Mesa 20.0.0-devel ======================================================= [build] use-vbo=false: FPS: 26 FrameTime: 38.462 ms [build] use-vbo=true: FPS: 26 FrameTime: 38.462 ms [texture] texture-filter=nearest: FPS: 26 FrameTime: 38.462 ms [texture] texture-filter=linear: FPS: 26 FrameTime: 38.462 ms [texture] texture-filter=mipmap: FPS: 27 FrameTime: 37.037 ms [shading] shading=gouraud: FPS: 27 FrameTime: 37.037 ms [shading] shading=blinn-phong-inf: FPS: 27 FrameTime: 37.037 ms [shading] shading=phong: FPS: 27 FrameTime: 37.037 ms [shading] shading=cel: FPS: 26 FrameTime: 38.462 ms [bump] bump-render=high-poly: FPS: 27 FrameTime: 37.037 ms [bump] bump-render=normals: FPS: 27 FrameTime: 37.037 ms [bump] bump-render=height: FPS: 27 FrameTime: 37.037 ms [effect2d] kernel=0,1,0;1,-4,1;0,1,0;: FPS: 25 FrameTime: 40.000 ms [effect2d] kernel=1,1,1,1,1;1,1,1,1,1;1,1,1,1,1;: FPS: 26 FrameTime: 38.462 ms [pulsar] light=false:quads=5:texture=false: FPS: 26 FrameTime: 38.462 ms [desktop] blur-radius=5:effect=blur:passes=1:separable=true:windows=4: FPS: 26 FrameTime: 38.462 ms [desktop] effect=shadow:windows=4: FPS: 27 FrameTime: 37.037 ms ... Brian > > On 3/3/20 7:26 AM, Brian Masney wrote: > > On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: > > > Another thing: did you verify that the panel still runs at 60hz (and not > > > dropping frames to 30hz)? IIRC that was the behavior with lower clock. > > > > Yes, the panel is running at 60 HZ according to the Xorg log with > > Ville's patch applied: > > > > modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 > > 1920 1922 1924 1926 (115.6 kHz eP) > > > > I verified there's no underflow errors in dmesg. > > > > If I recall correctly, the clock speeds that was in your tree was set > > too low for the gpu_opp_table (that wouldn't cause this issue), but I > > seem to recall there were some other clock speed mismatches. The > > bandwidth requests weren't set on the RPM as well, so maybe that > > contributed to the problem. That's done upstream with the msm8974 > > interconnect driver: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c > > > > There's a separate known issue with 'pp done time out' errors that > > occur on the framebuffer that started upstream several months ago with > > the introduction of async commit support in the MSM driver. I tried > > working around this by enabling the autorefresh feature but it's not > > fully working yet and I hit a dead end since there's no docs available > > publicly for this. The grim details are at: > > > > https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ > > > > So I'm still OK with Ville's patch going in. > > > > Brian > > > > > > > > > > On 3/2/20 10:28 PM, Jonathan Marek wrote: > > > > > > > > On 3/2/20 10:13 PM, Brian Masney wrote: > > > > > On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: > > > > > > Hi, > > > > > > > > > > > > This is a command mode panel and the the msm/mdp5 driver uses > > > > > > the vrefresh > > > > > > field for the actual refresh rate, while the dotclock field is > > > > > > used for the > > > > > > DSI clocks. The dotclock needed to be a bit higher than > > > > > > necessary otherwise > > > > > > the panel would not work. > > > > > > > > > > > > If you want to get rid of the separate clock/vrefresh fields there would > > > > > > need to be some changes to msm driver. > > > > > > > > > > > > (note I hadn't made the patch with upstreaming in mind, the > > > > > > 150000 value is > > > > > > likely not optimal, just something that worked, this is something that > > > > > > should have been checked with the downstream driver) > > > > > > > > > > Is this the right clock frequency in the downstream MSM 3.4 kernel that > > > > > you're talking about? > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 > > > > > > > > > > > > > > > > > > No, I'm talking about the DSI clock (the driver for it is in > > > > drm/msm/dsi/). For a command mode panel the front/back porches aren't > > > > relevant, but the dsi pixel/byte clock need to be a bit higher than > > > > 1920x1080x60. Since 125498 is a little higher than 124416 that might be > > > > enough (there is also rounding of the clock values to consider). > > > > > > > > > I don't see any obvious clock values in the downstream command mode > > > > > panel configuration: > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 > > > > > > > > > > > > > > > Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 > > > > > and everything appears to be working fine. You can add my Tested-by if > > > > > you end up applying this. > > > > > > > > > > Tested-by: Brian Masney <masneyb@onstation.org> > > > > > > > > > > Brian > > > > > > > > > > > > > > > > On 3/2/20 3:34 PM, Ville Syrjala wrote: > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > > > > > The currently listed dotclock disagrees with the currently > > > > > > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > > > > > > > > > > > Someone tell me which (if either) of the dotclock or vreresh is > > > > > > > correct? > > > > > > > > > > > > > > Cc: Jonathan Marek <jonathan@marek.ca> > > > > > > > Cc: Brian Masney <masneyb@onstation.org> > > > > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > --- > > > > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > index b24fdf239440..f958d8dfd760 100644 > > > > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi > > > > > > > panasonic_vvx10f004b00 = { > > > > > > > }; > > > > > > > static const struct drm_display_mode lg_acx467akm_7_mode = { > > > > > > > - .clock = 150000, > > > > > > > + .clock = 125498, > > > > > > > .hdisplay = 1080, > > > > > > > .hsync_start = 1080 + 2, > > > > > > > .hsync_end = 1080 + 2 + 2, > > > > > > >
modetest should be printing "freq: 60.0Hz", so definitely something wrong there. Though I guess you have another problem since I would expect the patch to drop it to 30 and not 13.5. (FYI glmark-x11 isn't vsynced which is why I specifically mentioned glmark-drm) On 3/3/20 9:16 PM, Brian Masney wrote: > On Tue, Mar 03, 2020 at 08:04:05AM -0500, Jonathan Marek wrote: >> What Xorg prints doesn't mean anything. I don't think there will be errors >> in dmesg, you need to run something that does pageflips as fast as possible >> and see that the refresh rate is still 60. (modetest with -v, glmark-drm are >> examples) > > I assume that you mean modetest from > https://gitlab.freedesktop.org/mesa/drm/tree/master/tests/modetest ? > Here's the modeset connector information: > > id encoder status name size (mm) modes encoders > 32 31 connected DSI-1 62x110 1 31 > modes: > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > #0 1080x1920 71.71 1080 1082 1084 1086 1920 1922 1924 1926 150000 > flags: ; type: preferred, driver > > And the page flip results... > > $ modetest -v -s 32:1080x1920 > trying to open device 'msm'...done > setting mode 1080x1920-71.71Hz@XR24 on connectors 32, crtc 50 > failed to set gamma: Function not implemented > freq: 13.50Hz > freq: 13.51Hz > freq: 13.51Hz > > It's the same results with and without Ville's patch. > > Here's the beginning of the glmark2 results with the x11-gl flavor: > > ======================================================= > glmark2 2017.07 > ======================================================= > OpenGL Information > GL_VENDOR: freedreno > GL_RENDERER: FD330 > GL_VERSION: 3.1 Mesa 20.0.0-devel > ======================================================= > [build] use-vbo=false: FPS: 26 FrameTime: 38.462 ms > [build] use-vbo=true: FPS: 26 FrameTime: 38.462 ms > [texture] texture-filter=nearest: FPS: 26 FrameTime: 38.462 ms > [texture] texture-filter=linear: FPS: 26 FrameTime: 38.462 ms > [texture] texture-filter=mipmap: FPS: 27 FrameTime: 37.037 ms > [shading] shading=gouraud: FPS: 27 FrameTime: 37.037 ms > [shading] shading=blinn-phong-inf: FPS: 27 FrameTime: 37.037 ms > [shading] shading=phong: FPS: 27 FrameTime: 37.037 ms > [shading] shading=cel: FPS: 26 FrameTime: 38.462 ms > [bump] bump-render=high-poly: FPS: 27 FrameTime: 37.037 ms > [bump] bump-render=normals: FPS: 27 FrameTime: 37.037 ms > [bump] bump-render=height: FPS: 27 FrameTime: 37.037 ms > [effect2d] kernel=0,1,0;1,-4,1;0,1,0;: FPS: 25 FrameTime: 40.000 ms > [effect2d] kernel=1,1,1,1,1;1,1,1,1,1;1,1,1,1,1;: FPS: 26 FrameTime: > 38.462 ms > [pulsar] light=false:quads=5:texture=false: FPS: 26 FrameTime: 38.462 ms > [desktop] blur-radius=5:effect=blur:passes=1:separable=true:windows=4: > FPS: 26 FrameTime: 38.462 ms > [desktop] effect=shadow:windows=4: FPS: 27 FrameTime: 37.037 ms > ... > > Brian > > >> >> On 3/3/20 7:26 AM, Brian Masney wrote: >>> On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: >>>> Another thing: did you verify that the panel still runs at 60hz (and not >>>> dropping frames to 30hz)? IIRC that was the behavior with lower clock. >>> >>> Yes, the panel is running at 60 HZ according to the Xorg log with >>> Ville's patch applied: >>> >>> modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 >>> 1920 1922 1924 1926 (115.6 kHz eP) >>> >>> I verified there's no underflow errors in dmesg. >>> >>> If I recall correctly, the clock speeds that was in your tree was set >>> too low for the gpu_opp_table (that wouldn't cause this issue), but I >>> seem to recall there were some other clock speed mismatches. The >>> bandwidth requests weren't set on the RPM as well, so maybe that >>> contributed to the problem. That's done upstream with the msm8974 >>> interconnect driver: >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c >>> >>> There's a separate known issue with 'pp done time out' errors that >>> occur on the framebuffer that started upstream several months ago with >>> the introduction of async commit support in the MSM driver. I tried >>> working around this by enabling the autorefresh feature but it's not >>> fully working yet and I hit a dead end since there's no docs available >>> publicly for this. The grim details are at: >>> >>> https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ >>> >>> So I'm still OK with Ville's patch going in. >>> >>> Brian >>> >>> >>>> >>>> On 3/2/20 10:28 PM, Jonathan Marek wrote: >>>>> >>>>> On 3/2/20 10:13 PM, Brian Masney wrote: >>>>>> On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: >>>>>>> Hi, >>>>>>> >>>>>>> This is a command mode panel and the the msm/mdp5 driver uses >>>>>>> the vrefresh >>>>>>> field for the actual refresh rate, while the dotclock field is >>>>>>> used for the >>>>>>> DSI clocks. The dotclock needed to be a bit higher than >>>>>>> necessary otherwise >>>>>>> the panel would not work. >>>>>>> >>>>>>> If you want to get rid of the separate clock/vrefresh fields there would >>>>>>> need to be some changes to msm driver. >>>>>>> >>>>>>> (note I hadn't made the patch with upstreaming in mind, the >>>>>>> 150000 value is >>>>>>> likely not optimal, just something that worked, this is something that >>>>>>> should have been checked with the downstream driver) >>>>>> >>>>>> Is this the right clock frequency in the downstream MSM 3.4 kernel that >>>>>> you're talking about? >>>>>> >>>>>> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 >>>>>> >>>>>> >>>>> >>>>> No, I'm talking about the DSI clock (the driver for it is in >>>>> drm/msm/dsi/). For a command mode panel the front/back porches aren't >>>>> relevant, but the dsi pixel/byte clock need to be a bit higher than >>>>> 1920x1080x60. Since 125498 is a little higher than 124416 that might be >>>>> enough (there is also rounding of the clock values to consider). >>>>> >>>>>> I don't see any obvious clock values in the downstream command mode >>>>>> panel configuration: >>>>>> >>>>>> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 >>>>>> >>>>>> >>>>>> Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 >>>>>> and everything appears to be working fine. You can add my Tested-by if >>>>>> you end up applying this. >>>>>> >>>>>> Tested-by: Brian Masney <masneyb@onstation.org> >>>>>> >>>>>> Brian >>>>>> >>>>>> >>>>>>> On 3/2/20 3:34 PM, Ville Syrjala wrote: >>>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>>>>> >>>>>>>> The currently listed dotclock disagrees with the currently >>>>>>>> listed vrefresh rate. Change the dotclock to match the vrefresh. >>>>>>>> >>>>>>>> Someone tell me which (if either) of the dotclock or vreresh is >>>>>>>> correct? >>>>>>>> >>>>>>>> Cc: Jonathan Marek <jonathan@marek.ca> >>>>>>>> Cc: Brian Masney <masneyb@onstation.org> >>>>>>>> Cc: Linus Walleij <linus.walleij@linaro.org> >>>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>>>>> --- >>>>>>>> drivers/gpu/drm/panel/panel-simple.c | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> index b24fdf239440..f958d8dfd760 100644 >>>>>>>> --- a/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>> @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi >>>>>>>> panasonic_vvx10f004b00 = { >>>>>>>> }; >>>>>>>> static const struct drm_display_mode lg_acx467akm_7_mode = { >>>>>>>> - .clock = 150000, >>>>>>>> + .clock = 125498, >>>>>>>> .hdisplay = 1080, >>>>>>>> .hsync_start = 1080 + 2, >>>>>>>> .hsync_end = 1080 + 2 + 2, >>>>>>>>
On Tue, Mar 03, 2020 at 09:27:50PM -0500, Jonathan Marek wrote: > modetest should be printing "freq: 60.0Hz", so definitely something wrong > there. Though I guess you have another problem since I would expect the > patch to drop it to 30 and not 13.5. > > (FYI glmark-x11 isn't vsynced which is why I specifically mentioned > glmark-drm) I tried compiling the drm variant and it was complaining about some missing dependencies that I didn't see in Alpine Linux. I didn't try too hard since I'm a bit short on time at this point since I'm starting a new job on Monday and I have another side project that I want to finish before then. I suspect that the issue is caused by the introduction of the async commit support in the MSM driver that introduced the ping/pong timeouts. I'll try in a few weeks or so reverting those patches and see if that affects the speed. I'm still ok with Ville's patch going in given the existing slow state. There's no clear path forward right now for the autocommit patch that I linked to earlier in this thread. :( Brian > > On 3/3/20 9:16 PM, Brian Masney wrote: > > On Tue, Mar 03, 2020 at 08:04:05AM -0500, Jonathan Marek wrote: > > > What Xorg prints doesn't mean anything. I don't think there will be errors > > > in dmesg, you need to run something that does pageflips as fast as possible > > > and see that the refresh rate is still 60. (modetest with -v, glmark-drm are > > > examples) > > > > I assume that you mean modetest from > > https://gitlab.freedesktop.org/mesa/drm/tree/master/tests/modetest ? > > Here's the modeset connector information: > > > > id encoder status name size (mm) modes encoders > > 32 31 connected DSI-1 62x110 1 31 > > modes: > > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > > #0 1080x1920 71.71 1080 1082 1084 1086 1920 1922 1924 1926 150000 > > flags: ; type: preferred, driver > > > > And the page flip results... > > > > $ modetest -v -s 32:1080x1920 > > trying to open device 'msm'...done > > setting mode 1080x1920-71.71Hz@XR24 on connectors 32, crtc 50 > > failed to set gamma: Function not implemented > > freq: 13.50Hz > > freq: 13.51Hz > > freq: 13.51Hz > > > > It's the same results with and without Ville's patch. > > > > Here's the beginning of the glmark2 results with the x11-gl flavor: > > > > ======================================================= > > glmark2 2017.07 > > ======================================================= > > OpenGL Information > > GL_VENDOR: freedreno > > GL_RENDERER: FD330 > > GL_VERSION: 3.1 Mesa 20.0.0-devel > > ======================================================= > > [build] use-vbo=false: FPS: 26 FrameTime: 38.462 ms > > [build] use-vbo=true: FPS: 26 FrameTime: 38.462 ms > > [texture] texture-filter=nearest: FPS: 26 FrameTime: 38.462 ms > > [texture] texture-filter=linear: FPS: 26 FrameTime: 38.462 ms > > [texture] texture-filter=mipmap: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=gouraud: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=blinn-phong-inf: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=phong: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=cel: FPS: 26 FrameTime: 38.462 ms > > [bump] bump-render=high-poly: FPS: 27 FrameTime: 37.037 ms > > [bump] bump-render=normals: FPS: 27 FrameTime: 37.037 ms > > [bump] bump-render=height: FPS: 27 FrameTime: 37.037 ms > > [effect2d] kernel=0,1,0;1,-4,1;0,1,0;: FPS: 25 FrameTime: 40.000 ms > > [effect2d] kernel=1,1,1,1,1;1,1,1,1,1;1,1,1,1,1;: FPS: 26 FrameTime: > > 38.462 ms > > [pulsar] light=false:quads=5:texture=false: FPS: 26 FrameTime: 38.462 ms > > [desktop] blur-radius=5:effect=blur:passes=1:separable=true:windows=4: > > FPS: 26 FrameTime: 38.462 ms > > [desktop] effect=shadow:windows=4: FPS: 27 FrameTime: 37.037 ms > > ... > > > > Brian > > > > > > > > > > On 3/3/20 7:26 AM, Brian Masney wrote: > > > > On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: > > > > > Another thing: did you verify that the panel still runs at 60hz (and not > > > > > dropping frames to 30hz)? IIRC that was the behavior with lower clock. > > > > > > > > Yes, the panel is running at 60 HZ according to the Xorg log with > > > > Ville's patch applied: > > > > > > > > modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 > > > > 1920 1922 1924 1926 (115.6 kHz eP) > > > > > > > > I verified there's no underflow errors in dmesg. > > > > > > > > If I recall correctly, the clock speeds that was in your tree was set > > > > too low for the gpu_opp_table (that wouldn't cause this issue), but I > > > > seem to recall there were some other clock speed mismatches. The > > > > bandwidth requests weren't set on the RPM as well, so maybe that > > > > contributed to the problem. That's done upstream with the msm8974 > > > > interconnect driver: > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c > > > > > > > > There's a separate known issue with 'pp done time out' errors that > > > > occur on the framebuffer that started upstream several months ago with > > > > the introduction of async commit support in the MSM driver. I tried > > > > working around this by enabling the autorefresh feature but it's not > > > > fully working yet and I hit a dead end since there's no docs available > > > > publicly for this. The grim details are at: > > > > > > > > https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ > > > > > > > > So I'm still OK with Ville's patch going in. > > > > > > > > Brian > > > > > > > > > > > > > > > > > > On 3/2/20 10:28 PM, Jonathan Marek wrote: > > > > > > > > > > > > On 3/2/20 10:13 PM, Brian Masney wrote: > > > > > > > On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > This is a command mode panel and the the msm/mdp5 driver uses > > > > > > > > the vrefresh > > > > > > > > field for the actual refresh rate, while the dotclock field is > > > > > > > > used for the > > > > > > > > DSI clocks. The dotclock needed to be a bit higher than > > > > > > > > necessary otherwise > > > > > > > > the panel would not work. > > > > > > > > > > > > > > > > If you want to get rid of the separate clock/vrefresh fields there would > > > > > > > > need to be some changes to msm driver. > > > > > > > > > > > > > > > > (note I hadn't made the patch with upstreaming in mind, the > > > > > > > > 150000 value is > > > > > > > > likely not optimal, just something that worked, this is something that > > > > > > > > should have been checked with the downstream driver) > > > > > > > > > > > > > > Is this the right clock frequency in the downstream MSM 3.4 kernel that > > > > > > > you're talking about? > > > > > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 > > > > > > > > > > > > > > > > > > > > > > > > > > No, I'm talking about the DSI clock (the driver for it is in > > > > > > drm/msm/dsi/). For a command mode panel the front/back porches aren't > > > > > > relevant, but the dsi pixel/byte clock need to be a bit higher than > > > > > > 1920x1080x60. Since 125498 is a little higher than 124416 that might be > > > > > > enough (there is also rounding of the clock values to consider). > > > > > > > > > > > > > I don't see any obvious clock values in the downstream command mode > > > > > > > panel configuration: > > > > > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 > > > > > > > > > > > > > > > > > > > > > Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 > > > > > > > and everything appears to be working fine. You can add my Tested-by if > > > > > > > you end up applying this. > > > > > > > > > > > > > > Tested-by: Brian Masney <masneyb@onstation.org> > > > > > > > > > > > > > > Brian > > > > > > > > > > > > > > > > > > > > > > On 3/2/20 3:34 PM, Ville Syrjala wrote: > > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > > > > > > > > > The currently listed dotclock disagrees with the currently > > > > > > > > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > > > > > > > > > > > > > > > Someone tell me which (if either) of the dotclock or vreresh is > > > > > > > > > correct? > > > > > > > > > > > > > > > > > > Cc: Jonathan Marek <jonathan@marek.ca> > > > > > > > > > Cc: Brian Masney <masneyb@onstation.org> > > > > > > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > --- > > > > > > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > index b24fdf239440..f958d8dfd760 100644 > > > > > > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi > > > > > > > > > panasonic_vvx10f004b00 = { > > > > > > > > > }; > > > > > > > > > static const struct drm_display_mode lg_acx467akm_7_mode = { > > > > > > > > > - .clock = 150000, > > > > > > > > > + .clock = 125498, > > > > > > > > > .hdisplay = 1080, > > > > > > > > > .hsync_start = 1080 + 2, > > > > > > > > > .hsync_end = 1080 + 2 + 2, > > > > > > > > >
If I have time to kill over the weekend I'll do a new rebase of my Nexus 5 patches (my last rebase was back in August on 5.2, and the panel was working correctly at 60Hz back then). Looked at it again and it does look like your glmark was vsynced (glmark explicitly disables vsync so I guess you have it force-enabled) since the results are all 26-27 (X works a bit differently and gets double the framerate somehow?) On 3/3/20 9:53 PM, Brian Masney wrote: > On Tue, Mar 03, 2020 at 09:27:50PM -0500, Jonathan Marek wrote: >> modetest should be printing "freq: 60.0Hz", so definitely something wrong >> there. Though I guess you have another problem since I would expect the >> patch to drop it to 30 and not 13.5. >> >> (FYI glmark-x11 isn't vsynced which is why I specifically mentioned >> glmark-drm) > > I tried compiling the drm variant and it was complaining about some > missing dependencies that I didn't see in Alpine Linux. I didn't try too > hard since I'm a bit short on time at this point since I'm starting a > new job on Monday and I have another side project that I want to finish > before then. > > I suspect that the issue is caused by the introduction of the async > commit support in the MSM driver that introduced the ping/pong timeouts. > I'll try in a few weeks or so reverting those patches and see if that > affects the speed. > > I'm still ok with Ville's patch going in given the existing slow state. > There's no clear path forward right now for the autocommit patch that I > linked to earlier in this thread. :( > > Brian > >> >> On 3/3/20 9:16 PM, Brian Masney wrote: >>> On Tue, Mar 03, 2020 at 08:04:05AM -0500, Jonathan Marek wrote: >>>> What Xorg prints doesn't mean anything. I don't think there will be errors >>>> in dmesg, you need to run something that does pageflips as fast as possible >>>> and see that the refresh rate is still 60. (modetest with -v, glmark-drm are >>>> examples) >>> >>> I assume that you mean modetest from >>> https://gitlab.freedesktop.org/mesa/drm/tree/master/tests/modetest ? >>> Here's the modeset connector information: >>> >>> id encoder status name size (mm) modes encoders >>> 32 31 connected DSI-1 62x110 1 31 >>> modes: >>> index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) >>> #0 1080x1920 71.71 1080 1082 1084 1086 1920 1922 1924 1926 150000 >>> flags: ; type: preferred, driver >>> >>> And the page flip results... >>> >>> $ modetest -v -s 32:1080x1920 >>> trying to open device 'msm'...done >>> setting mode 1080x1920-71.71Hz@XR24 on connectors 32, crtc 50 >>> failed to set gamma: Function not implemented >>> freq: 13.50Hz >>> freq: 13.51Hz >>> freq: 13.51Hz >>> >>> It's the same results with and without Ville's patch. >>> >>> Here's the beginning of the glmark2 results with the x11-gl flavor: >>> >>> ======================================================= >>> glmark2 2017.07 >>> ======================================================= >>> OpenGL Information >>> GL_VENDOR: freedreno >>> GL_RENDERER: FD330 >>> GL_VERSION: 3.1 Mesa 20.0.0-devel >>> ======================================================= >>> [build] use-vbo=false: FPS: 26 FrameTime: 38.462 ms >>> [build] use-vbo=true: FPS: 26 FrameTime: 38.462 ms >>> [texture] texture-filter=nearest: FPS: 26 FrameTime: 38.462 ms >>> [texture] texture-filter=linear: FPS: 26 FrameTime: 38.462 ms >>> [texture] texture-filter=mipmap: FPS: 27 FrameTime: 37.037 ms >>> [shading] shading=gouraud: FPS: 27 FrameTime: 37.037 ms >>> [shading] shading=blinn-phong-inf: FPS: 27 FrameTime: 37.037 ms >>> [shading] shading=phong: FPS: 27 FrameTime: 37.037 ms >>> [shading] shading=cel: FPS: 26 FrameTime: 38.462 ms >>> [bump] bump-render=high-poly: FPS: 27 FrameTime: 37.037 ms >>> [bump] bump-render=normals: FPS: 27 FrameTime: 37.037 ms >>> [bump] bump-render=height: FPS: 27 FrameTime: 37.037 ms >>> [effect2d] kernel=0,1,0;1,-4,1;0,1,0;: FPS: 25 FrameTime: 40.000 ms >>> [effect2d] kernel=1,1,1,1,1;1,1,1,1,1;1,1,1,1,1;: FPS: 26 FrameTime: >>> 38.462 ms >>> [pulsar] light=false:quads=5:texture=false: FPS: 26 FrameTime: 38.462 ms >>> [desktop] blur-radius=5:effect=blur:passes=1:separable=true:windows=4: >>> FPS: 26 FrameTime: 38.462 ms >>> [desktop] effect=shadow:windows=4: FPS: 27 FrameTime: 37.037 ms >>> ... >>> >>> Brian >>> >>> >>>> >>>> On 3/3/20 7:26 AM, Brian Masney wrote: >>>>> On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: >>>>>> Another thing: did you verify that the panel still runs at 60hz (and not >>>>>> dropping frames to 30hz)? IIRC that was the behavior with lower clock. >>>>> >>>>> Yes, the panel is running at 60 HZ according to the Xorg log with >>>>> Ville's patch applied: >>>>> >>>>> modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 >>>>> 1920 1922 1924 1926 (115.6 kHz eP) >>>>> >>>>> I verified there's no underflow errors in dmesg. >>>>> >>>>> If I recall correctly, the clock speeds that was in your tree was set >>>>> too low for the gpu_opp_table (that wouldn't cause this issue), but I >>>>> seem to recall there were some other clock speed mismatches. The >>>>> bandwidth requests weren't set on the RPM as well, so maybe that >>>>> contributed to the problem. That's done upstream with the msm8974 >>>>> interconnect driver: >>>>> >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c >>>>> >>>>> There's a separate known issue with 'pp done time out' errors that >>>>> occur on the framebuffer that started upstream several months ago with >>>>> the introduction of async commit support in the MSM driver. I tried >>>>> working around this by enabling the autorefresh feature but it's not >>>>> fully working yet and I hit a dead end since there's no docs available >>>>> publicly for this. The grim details are at: >>>>> >>>>> https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ >>>>> >>>>> So I'm still OK with Ville's patch going in. >>>>> >>>>> Brian >>>>> >>>>> >>>>>> >>>>>> On 3/2/20 10:28 PM, Jonathan Marek wrote: >>>>>>> >>>>>>> On 3/2/20 10:13 PM, Brian Masney wrote: >>>>>>>> On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> This is a command mode panel and the the msm/mdp5 driver uses >>>>>>>>> the vrefresh >>>>>>>>> field for the actual refresh rate, while the dotclock field is >>>>>>>>> used for the >>>>>>>>> DSI clocks. The dotclock needed to be a bit higher than >>>>>>>>> necessary otherwise >>>>>>>>> the panel would not work. >>>>>>>>> >>>>>>>>> If you want to get rid of the separate clock/vrefresh fields there would >>>>>>>>> need to be some changes to msm driver. >>>>>>>>> >>>>>>>>> (note I hadn't made the patch with upstreaming in mind, the >>>>>>>>> 150000 value is >>>>>>>>> likely not optimal, just something that worked, this is something that >>>>>>>>> should have been checked with the downstream driver) >>>>>>>> >>>>>>>> Is this the right clock frequency in the downstream MSM 3.4 kernel that >>>>>>>> you're talking about? >>>>>>>> >>>>>>>> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> No, I'm talking about the DSI clock (the driver for it is in >>>>>>> drm/msm/dsi/). For a command mode panel the front/back porches aren't >>>>>>> relevant, but the dsi pixel/byte clock need to be a bit higher than >>>>>>> 1920x1080x60. Since 125498 is a little higher than 124416 that might be >>>>>>> enough (there is also rounding of the clock values to consider). >>>>>>> >>>>>>>> I don't see any obvious clock values in the downstream command mode >>>>>>>> panel configuration: >>>>>>>> >>>>>>>> https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 >>>>>>>> >>>>>>>> >>>>>>>> Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 >>>>>>>> and everything appears to be working fine. You can add my Tested-by if >>>>>>>> you end up applying this. >>>>>>>> >>>>>>>> Tested-by: Brian Masney <masneyb@onstation.org> >>>>>>>> >>>>>>>> Brian >>>>>>>> >>>>>>>> >>>>>>>>> On 3/2/20 3:34 PM, Ville Syrjala wrote: >>>>>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>>>>>>> >>>>>>>>>> The currently listed dotclock disagrees with the currently >>>>>>>>>> listed vrefresh rate. Change the dotclock to match the vrefresh. >>>>>>>>>> >>>>>>>>>> Someone tell me which (if either) of the dotclock or vreresh is >>>>>>>>>> correct? >>>>>>>>>> >>>>>>>>>> Cc: Jonathan Marek <jonathan@marek.ca> >>>>>>>>>> Cc: Brian Masney <masneyb@onstation.org> >>>>>>>>>> Cc: Linus Walleij <linus.walleij@linaro.org> >>>>>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>>>>>>> --- >>>>>>>>>> drivers/gpu/drm/panel/panel-simple.c | 2 +- >>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c >>>>>>>>>> b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>>>> index b24fdf239440..f958d8dfd760 100644 >>>>>>>>>> --- a/drivers/gpu/drm/panel/panel-simple.c >>>>>>>>>> +++ b/drivers/gpu/drm/panel/panel-simple.c >>>>>>>>>> @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi >>>>>>>>>> panasonic_vvx10f004b00 = { >>>>>>>>>> }; >>>>>>>>>> static const struct drm_display_mode lg_acx467akm_7_mode = { >>>>>>>>>> - .clock = 150000, >>>>>>>>>> + .clock = 125498, >>>>>>>>>> .hdisplay = 1080, >>>>>>>>>> .hsync_start = 1080 + 2, >>>>>>>>>> .hsync_end = 1080 + 2 + 2, >>>>>>>>>>
On Mon, Mar 2, 2020 at 9:49 PM Jonathan Marek <jonathan@marek.ca> wrote: > This is a command mode panel and the the msm/mdp5 driver uses the > vrefresh field for the actual refresh rate, while the dotclock field is > used for the DSI clocks. The dotclock needed to be a bit higher than > necessary otherwise the panel would not work. I don't know if this predates the support for defining DSI clocks but for what we have in the kernel right now this is wrong. struct mipi_dsi_device contains: * @hs_rate: maximum lane frequency for high speed mode in hertz, this should * be set to the real limits of the hardware, zero is only accepted for * legacy drivers * @lp_rate: maximum lane frequency for low power mode in hertz, this should * be set to the real limits of the hardware, zero is only accepted for * legacy drivers The MDP driver should use these frequencies for a DSI command mode panel, and the panel driver should define them. These two clocks are/can be/should be completely orthogonal to the dotclock/pixelclock inside the panel, which is likely driven from its own crystal directly from the panel-internal framebuffer. Yours, Linus Walleij
On Tue, Mar 03, 2020 at 10:04:56PM -0500, Jonathan Marek wrote: > If I have time to kill over the weekend I'll do a new rebase of my Nexus 5 > patches (my last rebase was back in August on 5.2, and the panel was working > correctly at 60Hz back then). That would be great if you have time to look at the panel. My out-of-tree patches for this phone are at https://github.com/masneyb/linux/commits/v5.5-nexus5. A high-level description of those patches are on the cover letter: https://github.com/masneyb/nexus-5-upstream/blob/master/out-of-tree-patches/upstream-patches/v5.5/0000-cover-letter.patch A description of what works and what I've done upstream for this device is described at: https://masneyb.github.io/nexus-5-upstream/ Brian > > Looked at it again and it does look like your glmark was vsynced (glmark > explicitly disables vsync so I guess you have it force-enabled) since the > results are all 26-27 (X works a bit differently and gets double the > framerate somehow?) > > On 3/3/20 9:53 PM, Brian Masney wrote: > > On Tue, Mar 03, 2020 at 09:27:50PM -0500, Jonathan Marek wrote: > > > modetest should be printing "freq: 60.0Hz", so definitely something wrong > > > there. Though I guess you have another problem since I would expect the > > > patch to drop it to 30 and not 13.5. > > > > > > (FYI glmark-x11 isn't vsynced which is why I specifically mentioned > > > glmark-drm) > > > > I tried compiling the drm variant and it was complaining about some > > missing dependencies that I didn't see in Alpine Linux. I didn't try too > > hard since I'm a bit short on time at this point since I'm starting a > > new job on Monday and I have another side project that I want to finish > > before then. > > > > I suspect that the issue is caused by the introduction of the async > > commit support in the MSM driver that introduced the ping/pong timeouts. > > I'll try in a few weeks or so reverting those patches and see if that > > affects the speed. > > > > I'm still ok with Ville's patch going in given the existing slow state. > > There's no clear path forward right now for the autocommit patch that I > > linked to earlier in this thread. :( > > > > Brian > > > > > > > > On 3/3/20 9:16 PM, Brian Masney wrote: > > > > On Tue, Mar 03, 2020 at 08:04:05AM -0500, Jonathan Marek wrote: > > > > > What Xorg prints doesn't mean anything. I don't think there will be errors > > > > > in dmesg, you need to run something that does pageflips as fast as possible > > > > > and see that the refresh rate is still 60. (modetest with -v, glmark-drm are > > > > > examples) > > > > > > > > I assume that you mean modetest from > > > > https://gitlab.freedesktop.org/mesa/drm/tree/master/tests/modetest ? > > > > Here's the modeset connector information: > > > > > > > > id encoder status name size (mm) modes encoders > > > > 32 31 connected DSI-1 62x110 1 31 > > > > modes: > > > > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > > > > #0 1080x1920 71.71 1080 1082 1084 1086 1920 1922 1924 1926 150000 > > > > flags: ; type: preferred, driver > > > > > > > > And the page flip results... > > > > > > > > $ modetest -v -s 32:1080x1920 > > > > trying to open device 'msm'...done > > > > setting mode 1080x1920-71.71Hz@XR24 on connectors 32, crtc 50 > > > > failed to set gamma: Function not implemented > > > > freq: 13.50Hz > > > > freq: 13.51Hz > > > > freq: 13.51Hz > > > > > > > > It's the same results with and without Ville's patch. > > > > > > > > Here's the beginning of the glmark2 results with the x11-gl flavor: > > > > > > > > ======================================================= > > > > glmark2 2017.07 > > > > ======================================================= > > > > OpenGL Information > > > > GL_VENDOR: freedreno > > > > GL_RENDERER: FD330 > > > > GL_VERSION: 3.1 Mesa 20.0.0-devel > > > > ======================================================= > > > > [build] use-vbo=false: FPS: 26 FrameTime: 38.462 ms > > > > [build] use-vbo=true: FPS: 26 FrameTime: 38.462 ms > > > > [texture] texture-filter=nearest: FPS: 26 FrameTime: 38.462 ms > > > > [texture] texture-filter=linear: FPS: 26 FrameTime: 38.462 ms > > > > [texture] texture-filter=mipmap: FPS: 27 FrameTime: 37.037 ms > > > > [shading] shading=gouraud: FPS: 27 FrameTime: 37.037 ms > > > > [shading] shading=blinn-phong-inf: FPS: 27 FrameTime: 37.037 ms > > > > [shading] shading=phong: FPS: 27 FrameTime: 37.037 ms > > > > [shading] shading=cel: FPS: 26 FrameTime: 38.462 ms > > > > [bump] bump-render=high-poly: FPS: 27 FrameTime: 37.037 ms > > > > [bump] bump-render=normals: FPS: 27 FrameTime: 37.037 ms > > > > [bump] bump-render=height: FPS: 27 FrameTime: 37.037 ms > > > > [effect2d] kernel=0,1,0;1,-4,1;0,1,0;: FPS: 25 FrameTime: 40.000 ms > > > > [effect2d] kernel=1,1,1,1,1;1,1,1,1,1;1,1,1,1,1;: FPS: 26 FrameTime: > > > > 38.462 ms > > > > [pulsar] light=false:quads=5:texture=false: FPS: 26 FrameTime: 38.462 ms > > > > [desktop] blur-radius=5:effect=blur:passes=1:separable=true:windows=4: > > > > FPS: 26 FrameTime: 38.462 ms > > > > [desktop] effect=shadow:windows=4: FPS: 27 FrameTime: 37.037 ms > > > > ... > > > > > > > > Brian > > > > > > > > > > > > > > > > > > On 3/3/20 7:26 AM, Brian Masney wrote: > > > > > > On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: > > > > > > > Another thing: did you verify that the panel still runs at 60hz (and not > > > > > > > dropping frames to 30hz)? IIRC that was the behavior with lower clock. > > > > > > > > > > > > Yes, the panel is running at 60 HZ according to the Xorg log with > > > > > > Ville's patch applied: > > > > > > > > > > > > modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 > > > > > > 1920 1922 1924 1926 (115.6 kHz eP) > > > > > > > > > > > > I verified there's no underflow errors in dmesg. > > > > > > > > > > > > If I recall correctly, the clock speeds that was in your tree was set > > > > > > too low for the gpu_opp_table (that wouldn't cause this issue), but I > > > > > > seem to recall there were some other clock speed mismatches. The > > > > > > bandwidth requests weren't set on the RPM as well, so maybe that > > > > > > contributed to the problem. That's done upstream with the msm8974 > > > > > > interconnect driver: > > > > > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c > > > > > > > > > > > > There's a separate known issue with 'pp done time out' errors that > > > > > > occur on the framebuffer that started upstream several months ago with > > > > > > the introduction of async commit support in the MSM driver. I tried > > > > > > working around this by enabling the autorefresh feature but it's not > > > > > > fully working yet and I hit a dead end since there's no docs available > > > > > > publicly for this. The grim details are at: > > > > > > > > > > > > https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ > > > > > > > > > > > > So I'm still OK with Ville's patch going in. > > > > > > > > > > > > Brian > > > > > > > > > > > > > > > > > > > > > > > > > > On 3/2/20 10:28 PM, Jonathan Marek wrote: > > > > > > > > > > > > > > > > On 3/2/20 10:13 PM, Brian Masney wrote: > > > > > > > > > On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > This is a command mode panel and the the msm/mdp5 driver uses > > > > > > > > > > the vrefresh > > > > > > > > > > field for the actual refresh rate, while the dotclock field is > > > > > > > > > > used for the > > > > > > > > > > DSI clocks. The dotclock needed to be a bit higher than > > > > > > > > > > necessary otherwise > > > > > > > > > > the panel would not work. > > > > > > > > > > > > > > > > > > > > If you want to get rid of the separate clock/vrefresh fields there would > > > > > > > > > > need to be some changes to msm driver. > > > > > > > > > > > > > > > > > > > > (note I hadn't made the patch with upstreaming in mind, the > > > > > > > > > > 150000 value is > > > > > > > > > > likely not optimal, just something that worked, this is something that > > > > > > > > > > should have been checked with the downstream driver) > > > > > > > > > > > > > > > > > > Is this the right clock frequency in the downstream MSM 3.4 kernel that > > > > > > > > > you're talking about? > > > > > > > > > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, I'm talking about the DSI clock (the driver for it is in > > > > > > > > drm/msm/dsi/). For a command mode panel the front/back porches aren't > > > > > > > > relevant, but the dsi pixel/byte clock need to be a bit higher than > > > > > > > > 1920x1080x60. Since 125498 is a little higher than 124416 that might be > > > > > > > > enough (there is also rounding of the clock values to consider). > > > > > > > > > > > > > > > > > I don't see any obvious clock values in the downstream command mode > > > > > > > > > panel configuration: > > > > > > > > > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 > > > > > > > > > > > > > > > > > > > > > > > > > > > Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 > > > > > > > > > and everything appears to be working fine. You can add my Tested-by if > > > > > > > > > you end up applying this. > > > > > > > > > > > > > > > > > > Tested-by: Brian Masney <masneyb@onstation.org> > > > > > > > > > > > > > > > > > > Brian > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 3/2/20 3:34 PM, Ville Syrjala wrote: > > > > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > > > > > > > > > > > > > The currently listed dotclock disagrees with the currently > > > > > > > > > > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > > > > > > > > > > > > > > > > > > > Someone tell me which (if either) of the dotclock or vreresh is > > > > > > > > > > > correct? > > > > > > > > > > > > > > > > > > > > > > Cc: Jonathan Marek <jonathan@marek.ca> > > > > > > > > > > > Cc: Brian Masney <masneyb@onstation.org> > > > > > > > > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > > --- > > > > > > > > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > > > index b24fdf239440..f958d8dfd760 100644 > > > > > > > > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > > > @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi > > > > > > > > > > > panasonic_vvx10f004b00 = { > > > > > > > > > > > }; > > > > > > > > > > > static const struct drm_display_mode lg_acx467akm_7_mode = { > > > > > > > > > > > - .clock = 150000, > > > > > > > > > > > + .clock = 125498, > > > > > > > > > > > .hdisplay = 1080, > > > > > > > > > > > .hsync_start = 1080 + 2, > > > > > > > > > > > .hsync_end = 1080 + 2 + 2, > > > > > > > > > > >
On Tue, Mar 03, 2020 at 09:27:50PM -0500, Jonathan Marek wrote: > modetest should be printing "freq: 60.0Hz", so definitely something wrong > there. Though I guess you have another problem since I would expect the > patch to drop it to 30 and not 13.5. So I reverted the following three commits related to the async commit support in the MSM driver: d934a712c5e6 ("drm/msm: add atomic traces") 2d99ced787e3 ("drm/msm: async commit support") 194fc68d7a49 ("drm/msm/dpu: async commit support") The modetest results now look much better: # With existing 150000 clock rate on the panel $ modetest -v -s 32:1080x19 setting mode 1080x1920-71.71Hz@XR24 on connectors 32, crtc 50 failed to set gamma: Function not implemented freq: 59.40Hz freq: 59.69Hz freq: 59.69Hz ... With Ville's patch the frequency drops slightly. # With 125498 clock rate on the panel $ modetest -v -s 32:1080x19 freq: 55.44Hz freq: 55.09Hz freq: 55.88Hz ... Brian > > (FYI glmark-x11 isn't vsynced which is why I specifically mentioned > glmark-drm) > > On 3/3/20 9:16 PM, Brian Masney wrote: > > On Tue, Mar 03, 2020 at 08:04:05AM -0500, Jonathan Marek wrote: > > > What Xorg prints doesn't mean anything. I don't think there will be errors > > > in dmesg, you need to run something that does pageflips as fast as possible > > > and see that the refresh rate is still 60. (modetest with -v, glmark-drm are > > > examples) > > > > I assume that you mean modetest from > > https://gitlab.freedesktop.org/mesa/drm/tree/master/tests/modetest ? > > Here's the modeset connector information: > > > > id encoder status name size (mm) modes encoders > > 32 31 connected DSI-1 62x110 1 31 > > modes: > > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > > #0 1080x1920 71.71 1080 1082 1084 1086 1920 1922 1924 1926 150000 > > flags: ; type: preferred, driver > > > > And the page flip results... > > > > $ modetest -v -s 32:1080x1920 > > trying to open device 'msm'...done > > setting mode 1080x1920-71.71Hz@XR24 on connectors 32, crtc 50 > > failed to set gamma: Function not implemented > > freq: 13.50Hz > > freq: 13.51Hz > > freq: 13.51Hz > > > > It's the same results with and without Ville's patch. > > > > Here's the beginning of the glmark2 results with the x11-gl flavor: > > > > ======================================================= > > glmark2 2017.07 > > ======================================================= > > OpenGL Information > > GL_VENDOR: freedreno > > GL_RENDERER: FD330 > > GL_VERSION: 3.1 Mesa 20.0.0-devel > > ======================================================= > > [build] use-vbo=false: FPS: 26 FrameTime: 38.462 ms > > [build] use-vbo=true: FPS: 26 FrameTime: 38.462 ms > > [texture] texture-filter=nearest: FPS: 26 FrameTime: 38.462 ms > > [texture] texture-filter=linear: FPS: 26 FrameTime: 38.462 ms > > [texture] texture-filter=mipmap: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=gouraud: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=blinn-phong-inf: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=phong: FPS: 27 FrameTime: 37.037 ms > > [shading] shading=cel: FPS: 26 FrameTime: 38.462 ms > > [bump] bump-render=high-poly: FPS: 27 FrameTime: 37.037 ms > > [bump] bump-render=normals: FPS: 27 FrameTime: 37.037 ms > > [bump] bump-render=height: FPS: 27 FrameTime: 37.037 ms > > [effect2d] kernel=0,1,0;1,-4,1;0,1,0;: FPS: 25 FrameTime: 40.000 ms > > [effect2d] kernel=1,1,1,1,1;1,1,1,1,1;1,1,1,1,1;: FPS: 26 FrameTime: > > 38.462 ms > > [pulsar] light=false:quads=5:texture=false: FPS: 26 FrameTime: 38.462 ms > > [desktop] blur-radius=5:effect=blur:passes=1:separable=true:windows=4: > > FPS: 26 FrameTime: 38.462 ms > > [desktop] effect=shadow:windows=4: FPS: 27 FrameTime: 37.037 ms > > ... > > > > Brian > > > > > > > > > > On 3/3/20 7:26 AM, Brian Masney wrote: > > > > On Mon, Mar 02, 2020 at 10:36:54PM -0500, Jonathan Marek wrote: > > > > > Another thing: did you verify that the panel still runs at 60hz (and not > > > > > dropping frames to 30hz)? IIRC that was the behavior with lower clock. > > > > > > > > Yes, the panel is running at 60 HZ according to the Xorg log with > > > > Ville's patch applied: > > > > > > > > modeset(0): Modeline "1080x1920"x60.0 125.50 1080 1082 1084 1086 > > > > 1920 1922 1924 1926 (115.6 kHz eP) > > > > > > > > I verified there's no underflow errors in dmesg. > > > > > > > > If I recall correctly, the clock speeds that was in your tree was set > > > > too low for the gpu_opp_table (that wouldn't cause this issue), but I > > > > seem to recall there were some other clock speed mismatches. The > > > > bandwidth requests weren't set on the RPM as well, so maybe that > > > > contributed to the problem. That's done upstream with the msm8974 > > > > interconnect driver: > > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/interconnect/qcom/msm8974.c > > > > > > > > There's a separate known issue with 'pp done time out' errors that > > > > occur on the framebuffer that started upstream several months ago with > > > > the introduction of async commit support in the MSM driver. I tried > > > > working around this by enabling the autorefresh feature but it's not > > > > fully working yet and I hit a dead end since there's no docs available > > > > publicly for this. The grim details are at: > > > > > > > > https://lore.kernel.org/lkml/20191230020053.26016-2-masneyb@onstation.org/ > > > > > > > > So I'm still OK with Ville's patch going in. > > > > > > > > Brian > > > > > > > > > > > > > > > > > > On 3/2/20 10:28 PM, Jonathan Marek wrote: > > > > > > > > > > > > On 3/2/20 10:13 PM, Brian Masney wrote: > > > > > > > On Mon, Mar 02, 2020 at 03:48:22PM -0500, Jonathan Marek wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > This is a command mode panel and the the msm/mdp5 driver uses > > > > > > > > the vrefresh > > > > > > > > field for the actual refresh rate, while the dotclock field is > > > > > > > > used for the > > > > > > > > DSI clocks. The dotclock needed to be a bit higher than > > > > > > > > necessary otherwise > > > > > > > > the panel would not work. > > > > > > > > > > > > > > > > If you want to get rid of the separate clock/vrefresh fields there would > > > > > > > > need to be some changes to msm driver. > > > > > > > > > > > > > > > > (note I hadn't made the patch with upstreaming in mind, the > > > > > > > > 150000 value is > > > > > > > > likely not optimal, just something that worked, this is something that > > > > > > > > should have been checked with the downstream driver) > > > > > > > > > > > > > > Is this the right clock frequency in the downstream MSM 3.4 kernel that > > > > > > > you're talking about? > > > > > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/mach-msm/clock-8974.c#L3326 > > > > > > > > > > > > > > > > > > > > > > > > > > No, I'm talking about the DSI clock (the driver for it is in > > > > > > drm/msm/dsi/). For a command mode panel the front/back porches aren't > > > > > > relevant, but the dsi pixel/byte clock need to be a bit higher than > > > > > > 1920x1080x60. Since 125498 is a little higher than 124416 that might be > > > > > > enough (there is also rounding of the clock values to consider). > > > > > > > > > > > > > I don't see any obvious clock values in the downstream command mode > > > > > > > panel configuration: > > > > > > > > > > > > > > https://github.com/AICP/kernel_lge_hammerhead/blob/n7.1/arch/arm/boot/dts/msm8974-hammerhead/msm8974-hammerhead-panel.dtsi#L591 > > > > > > > > > > > > > > > > > > > > > Anyways, I tried Ville's patch with the framebuffer, kmscube, and X11 > > > > > > > and everything appears to be working fine. You can add my Tested-by if > > > > > > > you end up applying this. > > > > > > > > > > > > > > Tested-by: Brian Masney <masneyb@onstation.org> > > > > > > > > > > > > > > Brian > > > > > > > > > > > > > > > > > > > > > > On 3/2/20 3:34 PM, Ville Syrjala wrote: > > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > > > > > > > > > > The currently listed dotclock disagrees with the currently > > > > > > > > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > > > > > > > > > > > > > > > Someone tell me which (if either) of the dotclock or vreresh is > > > > > > > > > correct? > > > > > > > > > > > > > > > > > > Cc: Jonathan Marek <jonathan@marek.ca> > > > > > > > > > Cc: Brian Masney <masneyb@onstation.org> > > > > > > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > > --- > > > > > > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > index b24fdf239440..f958d8dfd760 100644 > > > > > > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > > @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi > > > > > > > > > panasonic_vvx10f004b00 = { > > > > > > > > > }; > > > > > > > > > static const struct drm_display_mode lg_acx467akm_7_mode = { > > > > > > > > > - .clock = 150000, > > > > > > > > > + .clock = 125498, > > > > > > > > > .hdisplay = 1080, > > > > > > > > > .hsync_start = 1080 + 2, > > > > > > > > > .hsync_end = 1080 + 2 + 2, > > > > > > > > >
The msm DSI driver does predate the addition of those fields and doesn't use them at all. Seems like it would be a bit of a hack too, since the frequency we want to use is not the "real limits of the hardware".. On 3/4/20 4:10 AM, Linus Walleij wrote: > On Mon, Mar 2, 2020 at 9:49 PM Jonathan Marek <jonathan@marek.ca> wrote: > >> This is a command mode panel and the the msm/mdp5 driver uses the >> vrefresh field for the actual refresh rate, while the dotclock field is >> used for the DSI clocks. The dotclock needed to be a bit higher than >> necessary otherwise the panel would not work. > > I don't know if this predates the support for defining DSI clocks > but for what we have in the kernel right now this is wrong. > > struct mipi_dsi_device contains: > > * @hs_rate: maximum lane frequency for high speed mode in hertz, this should > * be set to the real limits of the hardware, zero is only accepted for > * legacy drivers > * @lp_rate: maximum lane frequency for low power mode in hertz, this should > * be set to the real limits of the hardware, zero is only accepted for > * legacy drivers > > The MDP driver should use these frequencies for a DSI command > mode panel, and the panel driver should define them. > > These two clocks are/can be/should be completely orthogonal to > the dotclock/pixelclock inside the panel, which is likely driven from > its own crystal directly from the panel-internal framebuffer. > > Yours, > Linus Walleij >
On Wed, Mar 4, 2020 at 2:17 PM Jonathan Marek <jonathan@marek.ca> wrote: [hs_rate / lp_rate] > The msm DSI driver does predate the addition of those fields and doesn't > use them at all. I think it would be benficient to switch to these fields, because then the .clock field (dot/pixelclock) is not abused to contain what I guess is the desires hs_rate. > Seems like it would be a bit of a hack too, since the frequency we want > to use is not the "real limits of the hardware".. That just means "clock it as high as the panel can take". Normally that would come from the vendor datasheet of the panel. If you don't have the datasheet, whatever you use in the vendor tree is fine, I suppose what is currently in .clock is fine. Yours, Linus Walleij
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index b24fdf239440..f958d8dfd760 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -3996,7 +3996,7 @@ static const struct panel_desc_dsi panasonic_vvx10f004b00 = { }; static const struct drm_display_mode lg_acx467akm_7_mode = { - .clock = 150000, + .clock = 125498, .hdisplay = 1080, .hsync_start = 1080 + 2, .hsync_end = 1080 + 2 + 2,