| Message ID | 20170220175331.GC963@e110455-lin.cambridge.arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 20, 2017 at 05:53:31PM +0000, Liviu Dudau wrote: > Sorry, looks like this fell through the cracks of us trying to fix the > other issues you were seeing. I'm attaching a patch, please let me know > if this works for you. I don't have time to test right now, but it looks similar to what I had. Only nit is - is dest_w used apart from being written once? (That's probably something for a separate patch.) There's also the issue which Daniel Vetter followed up with to my comment about this calculation - the plane state validation is insufficient, and it would be a good idea if it used drm_plane_helper_check_state(). Also, I've since noticed that it doesn't check state->rotation, and I don't think this driver supports any rotation options.
On Mon, Feb 20, 2017 at 05:53:31PM +0000, Liviu Dudau wrote: > Hi Russell, > > On Mon, Feb 20, 2017 at 12:16:03PM +0000, Russell King - ARM Linux wrote: > > On Fri, Nov 18, 2016 at 11:37:33PM +0000, Russell King - ARM Linux wrote: > > > Something I also noticed is this: > > > > > > scanout_start = gem->paddr + plane->state->fb->offsets[0] + > > > plane->state->crtc_y * plane->state->fb->pitches[0] + > > > plane->state->crtc_x * bpp / 8; > > > > > > Surely this should be using src_[xy] (which are the position in the > > > source - iow, memory, and not crtc_[xy] which is the position on the > > > CRTC displayed window. To put it another way, the src_* define the > > > region of the source material that is mapped onto a rectangular area > > > on the display defined by crtc_*. > > > > This problem still exists... > > Sorry, looks like this fell through the cracks of us trying to fix the > other issues you were seeing. I'm attaching a patch, please let me know > if this works for you. > > Best regards, > Liviu > > > > > -- > > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > > according to speedtest.net. > > -- > ==================== > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --------------- > ¯\_(ツ)_/¯ > >From a495d262cb91fdeccb00685646ddb71774c18e7e Mon Sep 17 00:00:00 2001 > From: Liviu Dudau <Liviu.Dudau@arm.com> > Date: Mon, 20 Feb 2017 17:44:42 +0000 > Subject: [PATCH] arm: hdlcd: Fix the calculation of scanout start address > > The framebuffer start address uses the CRTC's x,y position rather > than the source framebuffer's. Fix that. > > Reported-by: Russell King <rmk+kernel@armlinux.org.uk> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > --- > drivers/gpu/drm/arm/hdlcd_crtc.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c > index 798a3cc480a2..4c1ab73d9e07 100644 > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c > @@ -244,21 +244,20 @@ static void hdlcd_plane_atomic_update(struct drm_plane *plane, > struct drm_framebuffer *fb = plane->state->fb; > struct hdlcd_drm_private *hdlcd; > struct drm_gem_cma_object *gem; > - u32 src_w, src_h, dest_w, dest_h; > + u32 src_x, src_y, dest_w, dest_h; > dma_addr_t scanout_start; > > if (!fb) > return; > > - src_w = plane->state->src_w >> 16; > - src_h = plane->state->src_h >> 16; > + src_x = plane->state->src_x >> 16; > + src_y = plane->state->src_y >> 16; This stuff should be using the clipped coordinates, not the user coordinates. And it doesn't look like this guy is even calling the clip helper thing. malidp seems to be calling that thing, but it still doesn't manage to program the hw with the right coordinates from what I can see. /me feels a bit like a broken record... > dest_w = plane->state->crtc_w; > dest_h = plane->state->crtc_h; > gem = drm_fb_cma_get_gem_obj(fb, 0); > scanout_start = gem->paddr + fb->offsets[0] + > - plane->state->crtc_y * fb->pitches[0] + > - plane->state->crtc_x * > - fb->format->cpp[0]; > + src_y * fb->pitches[0] + > + src_x * fb->format->cpp[0]; > > hdlcd = plane->dev->dev_private; > hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, fb->pitches[0]); > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Feb 20, 2017 at 08:05:58PM +0200, Ville Syrjälä wrote: > This stuff should be using the clipped coordinates, not the user > coordinates. And it doesn't look like this guy is even calling the > clip helper thing. > > malidp seems to be calling that thing, but it still doesn't > manage to program the hw with the right coordinates from what > I can see. > > /me feels a bit like a broken record... If you mean drm_plane_helper_check_state(), then... $ grep drm_plane_helper_check_state Documentation/gpu/ -r So nothing there... but in drivers/gpu/drm/drm_plane_helper.c, there's the following, and I think this really isn't helping people understand what's required: * This helper library has two parts. The first part has support to implement * primary plane support on top of the normal CRTC configuration interface. * Since the legacy ->set_config interface ties the primary plane together with * the CRTC state this does not allow userspace to disable the primary plane * itself. To avoid too much duplicated code use * drm_plane_helper_check_update() which can be used to enforce the same * restrictions as primary planes had thus. The default primary plane only * expose XRBG8888 and ARGB8888 as valid pixel formats for the attached * framebuffer. * * Drivers are highly recommended to implement proper support for primary * planes, and newly merged drivers must not rely upon these transitional * helpers. Which functions are defined as "these transitional helpers" - the above is rather ambiguous. Is drm_plane_helper_check_state() a "transitional helper" or is it not? (It probably isn't, but the documentation does not make that clear.) It then goes on to: * The second part also implements transitional helpers which allow drivers to So maybe the second paragraph needs to be moved after this line to remove the confusion? If you find that you're repeating something to many people, it's always a good idea to re-read the documentation that's supposed to be giving people guidance. Now, when you say that we're supposed to program "clipped coordinates" maybe you can give a hint what those are and where they come from? Is that the vaguely documented "clip" parameter in drm_plane_helper_check_state() ? * @clip: integer clipping coordinates If it is, that doesn't really describe it, and neither does the description of what the function does, nor what it returns: * Checks that a desired plane update is valid. Drivers that provide * their own plane handling rather than helper-provided implementations may * still wish to call this function to avoid duplication of error checking * code. * * RETURNS: * Zero if update appears valid, error code on failure So, some improvement there could go a long way towards eliminating some of these issues... Atomic modeset is hideously complex... having poor documentation doesn't help.
Hi Ville, On Mon, Feb 20, 2017 at 08:05:58PM +0200, Ville Syrjälä wrote: > On Mon, Feb 20, 2017 at 05:53:31PM +0000, Liviu Dudau wrote: > > Hi Russell, > > > > On Mon, Feb 20, 2017 at 12:16:03PM +0000, Russell King - ARM Linux wrote: > > > On Fri, Nov 18, 2016 at 11:37:33PM +0000, Russell King - ARM Linux wrote: > > > > Something I also noticed is this: > > > > > > > > scanout_start = gem->paddr + plane->state->fb->offsets[0] + > > > > plane->state->crtc_y * plane->state->fb->pitches[0] + > > > > plane->state->crtc_x * bpp / 8; > > > > > > > > Surely this should be using src_[xy] (which are the position in the > > > > source - iow, memory, and not crtc_[xy] which is the position on the > > > > CRTC displayed window. To put it another way, the src_* define the > > > > region of the source material that is mapped onto a rectangular area > > > > on the display defined by crtc_*. > > > > > > This problem still exists... > > > > Sorry, looks like this fell through the cracks of us trying to fix the > > other issues you were seeing. I'm attaching a patch, please let me know > > if this works for you. > > > > Best regards, > > Liviu > > > > > > > > -- > > > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > > > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > > > according to speedtest.net. > > > >From a495d262cb91fdeccb00685646ddb71774c18e7e Mon Sep 17 00:00:00 2001 > > From: Liviu Dudau <Liviu.Dudau@arm.com> > > Date: Mon, 20 Feb 2017 17:44:42 +0000 > > Subject: [PATCH] arm: hdlcd: Fix the calculation of scanout start address > > > > The framebuffer start address uses the CRTC's x,y position rather > > than the source framebuffer's. Fix that. > > > > Reported-by: Russell King <rmk+kernel@armlinux.org.uk> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > > --- > > drivers/gpu/drm/arm/hdlcd_crtc.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c > > index 798a3cc480a2..4c1ab73d9e07 100644 > > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c > > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c > > @@ -244,21 +244,20 @@ static void hdlcd_plane_atomic_update(struct drm_plane *plane, > > struct drm_framebuffer *fb = plane->state->fb; > > struct hdlcd_drm_private *hdlcd; > > struct drm_gem_cma_object *gem; > > - u32 src_w, src_h, dest_w, dest_h; > > + u32 src_x, src_y, dest_w, dest_h; > > dma_addr_t scanout_start; > > > > if (!fb) > > return; > > > > - src_w = plane->state->src_w >> 16; > > - src_h = plane->state->src_h >> 16; > > + src_x = plane->state->src_x >> 16; > > + src_y = plane->state->src_y >> 16; While still waiting for your reply to Russell's comments ... > > This stuff should be using the clipped coordinates, not the user > coordinates. And it doesn't look like this guy is even calling the > clip helper thing. That's right, the code I believe predates the existence of the clip helper thing (me reads it as "drm_plane_helper_check_state"), and I have not got around to update the code here. Before I do that I would like to get a bit more clarification on what you mean by the clipped coordinates, given that HDLCD doesn't support dirty buffer updates. > > malidp seems to be calling that thing, but it still doesn't > manage to program the hw with the right coordinates from what > I can see. would like more clarity on what the right coordinates should be. > > /me feels a bit like a broken record... Sorry, I was not aware of your previous interventions on the subject of "right coordinates". Could you please point me in the right direction, if it is not much trouble? Best regards, Liviu > > > dest_w = plane->state->crtc_w; > > dest_h = plane->state->crtc_h; > > gem = drm_fb_cma_get_gem_obj(fb, 0); > > scanout_start = gem->paddr + fb->offsets[0] + > > - plane->state->crtc_y * fb->pitches[0] + > > - plane->state->crtc_x * > > - fb->format->cpp[0]; > > + src_y * fb->pitches[0] + > > + src_x * fb->format->cpp[0]; > > > > hdlcd = plane->dev->dev_private; > > hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, fb->pitches[0]); > > -- > > 2.11.0 > > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > -- > Ville Syrjälä > Intel OTC
On Wed, Feb 22, 2017 at 03:15:42PM +0000, Liviu Dudau wrote: > Hi Ville, > > On Mon, Feb 20, 2017 at 08:05:58PM +0200, Ville Syrjälä wrote: > > On Mon, Feb 20, 2017 at 05:53:31PM +0000, Liviu Dudau wrote: > > > Hi Russell, > > > > > > On Mon, Feb 20, 2017 at 12:16:03PM +0000, Russell King - ARM Linux wrote: > > > > On Fri, Nov 18, 2016 at 11:37:33PM +0000, Russell King - ARM Linux wrote: > > > > > Something I also noticed is this: > > > > > > > > > > scanout_start = gem->paddr + plane->state->fb->offsets[0] + > > > > > plane->state->crtc_y * plane->state->fb->pitches[0] + > > > > > plane->state->crtc_x * bpp / 8; > > > > > > > > > > Surely this should be using src_[xy] (which are the position in the > > > > > source - iow, memory, and not crtc_[xy] which is the position on the > > > > > CRTC displayed window. To put it another way, the src_* define the > > > > > region of the source material that is mapped onto a rectangular area > > > > > on the display defined by crtc_*. > > > > > > > > This problem still exists... > > > > > > Sorry, looks like this fell through the cracks of us trying to fix the > > > other issues you were seeing. I'm attaching a patch, please let me know > > > if this works for you. > > > > > > Best regards, > > > Liviu > > > > > > > > > > > -- > > > > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > > > > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > > > > according to speedtest.net. > > > > > >From a495d262cb91fdeccb00685646ddb71774c18e7e Mon Sep 17 00:00:00 2001 > > > From: Liviu Dudau <Liviu.Dudau@arm.com> > > > Date: Mon, 20 Feb 2017 17:44:42 +0000 > > > Subject: [PATCH] arm: hdlcd: Fix the calculation of scanout start address > > > > > > The framebuffer start address uses the CRTC's x,y position rather > > > than the source framebuffer's. Fix that. > > > > > > Reported-by: Russell King <rmk+kernel@armlinux.org.uk> > > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > > > --- > > > drivers/gpu/drm/arm/hdlcd_crtc.c | 11 +++++------ > > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c > > > index 798a3cc480a2..4c1ab73d9e07 100644 > > > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c > > > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c > > > @@ -244,21 +244,20 @@ static void hdlcd_plane_atomic_update(struct drm_plane *plane, > > > struct drm_framebuffer *fb = plane->state->fb; > > > struct hdlcd_drm_private *hdlcd; > > > struct drm_gem_cma_object *gem; > > > - u32 src_w, src_h, dest_w, dest_h; > > > + u32 src_x, src_y, dest_w, dest_h; > > > dma_addr_t scanout_start; > > > > > > if (!fb) > > > return; > > > > > > - src_w = plane->state->src_w >> 16; > > > - src_h = plane->state->src_h >> 16; > > > + src_x = plane->state->src_x >> 16; > > > + src_y = plane->state->src_y >> 16; > > While still waiting for your reply to Russell's comments ... > > > > > This stuff should be using the clipped coordinates, not the user > > coordinates. And it doesn't look like this guy is even calling the > > clip helper thing. > > That's right, the code I believe predates the existence of the clip helper > thing (me reads it as "drm_plane_helper_check_state"), and I have not got > around to update the code here. Before I do that I would like to get a bit > more clarification on what you mean by the clipped coordinates, given > that HDLCD doesn't support dirty buffer updates. The requirement to handle out of bounds plane coordinates correctly was always there even before the helper existed. It's just that most people who wrote new drivers neglected to read the reference code (i915). Granted, it would be better to have actual documentation, and now that we have the clipped coordinates in the core state and we have some helpers we should have a sensible place for said documentation as well. > > > > > malidp seems to be calling that thing, but it still doesn't > > manage to program the hw with the right coordinates from what > > I can see. > > would like more clarity on what the right coordinates should be. state->src and state->dst Well, that's assuming that you can't directly feed negative/large coordinates to your hardware. > > > > > /me feels a bit like a broken record... > > Sorry, I was not aware of your previous interventions on the subject of "right > coordinates". Could you please point me in the right direction, if it is not > much trouble? > > Best regards, > Liviu > > > > > > dest_w = plane->state->crtc_w; > > > dest_h = plane->state->crtc_h; > > > gem = drm_fb_cma_get_gem_obj(fb, 0); > > > scanout_start = gem->paddr + fb->offsets[0] + > > > - plane->state->crtc_y * fb->pitches[0] + > > > - plane->state->crtc_x * > > > - fb->format->cpp[0]; > > > + src_y * fb->pitches[0] + > > > + src_x * fb->format->cpp[0]; > > > > > > hdlcd = plane->dev->dev_private; > > > hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, fb->pitches[0]); > > > -- > > > 2.11.0 > > > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > -- > > Ville Syrjälä > > Intel OTC > > -- > ==================== > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --------------- > ¯\_(ツ)_/¯
On Mon, Feb 20, 2017 at 06:59:49PM +0000, Russell King - ARM Linux wrote: > On Mon, Feb 20, 2017 at 08:05:58PM +0200, Ville Syrjälä wrote: > > This stuff should be using the clipped coordinates, not the user > > coordinates. And it doesn't look like this guy is even calling the > > clip helper thing. > > > > malidp seems to be calling that thing, but it still doesn't > > manage to program the hw with the right coordinates from what > > I can see. > > > > /me feels a bit like a broken record... > > If you mean drm_plane_helper_check_state(), then... > > $ grep drm_plane_helper_check_state Documentation/gpu/ -r > > So nothing there... but in drivers/gpu/drm/drm_plane_helper.c, there's > the following, and I think this really isn't helping people understand > what's required: > > * This helper library has two parts. The first part has support to implement > * primary plane support on top of the normal CRTC configuration interface. > * Since the legacy ->set_config interface ties the primary plane together with > * the CRTC state this does not allow userspace to disable the primary plane > * itself. To avoid too much duplicated code use > * drm_plane_helper_check_update() which can be used to enforce the same > * restrictions as primary planes had thus. The default primary plane only > * expose XRBG8888 and ARGB8888 as valid pixel formats for the attached > * framebuffer. > * > * Drivers are highly recommended to implement proper support for primary > * planes, and newly merged drivers must not rely upon these transitional > * helpers. > > Which functions are defined as "these transitional helpers" - the above > is rather ambiguous. Is drm_plane_helper_check_state() a "transitional > helper" or is it not? (It probably isn't, but the documentation does not > make that clear.) Nope. And I guess we might want to move it into some atomic code instead. IIRC Daniel even suggested that but I was too lazy to do it at the time. > > It then goes on to: > > * The second part also implements transitional helpers which allow drivers to > > So maybe the second paragraph needs to be moved after this line to > remove the confusion? > > If you find that you're repeating something to many people, it's always > a good idea to re-read the documentation that's supposed to be giving > people guidance. Docs are such a new thing. I've not ever read them through myself TBH. > > Now, when you say that we're supposed to program "clipped coordinates" > maybe you can give a hint what those are and where they come from? > Is that the vaguely documented "clip" parameter in > drm_plane_helper_check_state() ? > > * @clip: integer clipping coordinatesa /** * struct drm_plane_state - mutable plane state ... * @src: clipped source coordinates of the plane (in 16.16) * @dst: clipped destination coordinates of the plane > > If it is, that doesn't really describe it, and neither does the > description of what the function does, nor what it returns: > > * Checks that a desired plane update is valid. Drivers that provide > * their own plane handling rather than helper-provided implementations may > * still wish to call this function to avoid duplication of error checking > * code. > * > * RETURNS: > * Zero if update appears valid, error code on failure > > So, some improvement there could go a long way towards eliminating > some of these issues... > > Atomic modeset is hideously complex... having poor documentation doesn't > help. > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net.
On Wed, Feb 22, 2017 at 05:42:30PM +0200, Ville Syrjälä wrote: > On Mon, Feb 20, 2017 at 06:59:49PM +0000, Russell King - ARM Linux wrote: > > On Mon, Feb 20, 2017 at 08:05:58PM +0200, Ville Syrjälä wrote: > > > This stuff should be using the clipped coordinates, not the user > > > coordinates. And it doesn't look like this guy is even calling the > > > clip helper thing. > > > > > > malidp seems to be calling that thing, but it still doesn't > > > manage to program the hw with the right coordinates from what > > > I can see. > > > > > > /me feels a bit like a broken record... > > > > If you mean drm_plane_helper_check_state(), then... > > > > $ grep drm_plane_helper_check_state Documentation/gpu/ -r > > > > So nothing there... but in drivers/gpu/drm/drm_plane_helper.c, there's > > the following, and I think this really isn't helping people understand > > what's required: > > > > * This helper library has two parts. The first part has support to implement > > * primary plane support on top of the normal CRTC configuration interface. > > * Since the legacy ->set_config interface ties the primary plane together with > > * the CRTC state this does not allow userspace to disable the primary plane > > * itself. To avoid too much duplicated code use > > * drm_plane_helper_check_update() which can be used to enforce the same > > * restrictions as primary planes had thus. The default primary plane only > > * expose XRBG8888 and ARGB8888 as valid pixel formats for the attached > > * framebuffer. > > * > > * Drivers are highly recommended to implement proper support for primary > > * planes, and newly merged drivers must not rely upon these transitional > > * helpers. > > > > Which functions are defined as "these transitional helpers" - the above > > is rather ambiguous. Is drm_plane_helper_check_state() a "transitional > > helper" or is it not? (It probably isn't, but the documentation does not > > make that clear.) > > Nope. And I guess we might want to move it into some atomic code > instead. IIRC Daniel even suggested that but I was too lazy to do it at > the time. Yeah, this is a half-finished job with docs not really updated, and most drivers also not really updated. I guess time to at least note this in our shiny new todo.rst file. I'll type a patch. -Daniel
From a495d262cb91fdeccb00685646ddb71774c18e7e Mon Sep 17 00:00:00 2001 From: Liviu Dudau <Liviu.Dudau@arm.com> Date: Mon, 20 Feb 2017 17:44:42 +0000 Subject: [PATCH] arm: hdlcd: Fix the calculation of scanout start address The framebuffer start address uses the CRTC's x,y position rather than the source framebuffer's. Fix that. Reported-by: Russell King <rmk+kernel@armlinux.org.uk> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> --- drivers/gpu/drm/arm/hdlcd_crtc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c index 798a3cc480a2..4c1ab73d9e07 100644 --- a/drivers/gpu/drm/arm/hdlcd_crtc.c +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c @@ -244,21 +244,20 @@ static void hdlcd_plane_atomic_update(struct drm_plane *plane, struct drm_framebuffer *fb = plane->state->fb; struct hdlcd_drm_private *hdlcd; struct drm_gem_cma_object *gem; - u32 src_w, src_h, dest_w, dest_h; + u32 src_x, src_y, dest_w, dest_h; dma_addr_t scanout_start; if (!fb) return; - src_w = plane->state->src_w >> 16; - src_h = plane->state->src_h >> 16; + src_x = plane->state->src_x >> 16; + src_y = plane->state->src_y >> 16; dest_w = plane->state->crtc_w; dest_h = plane->state->crtc_h; gem = drm_fb_cma_get_gem_obj(fb, 0); scanout_start = gem->paddr + fb->offsets[0] + - plane->state->crtc_y * fb->pitches[0] + - plane->state->crtc_x * - fb->format->cpp[0]; + src_y * fb->pitches[0] + + src_x * fb->format->cpp[0]; hdlcd = plane->dev->dev_private; hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, fb->pitches[0]); -- 2.11.0