Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Mon, Sep 03, 2018 at 09:26:24AM +0200, Daniel Vetter wrote:
> On Fri, Aug 31, 2018 at 05:26:37PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > Hi,
> >
> > On Fri, Aug 31, 2018 at 05:12:24PM +0200, Daniel Vetter wrote:
> > > On Fri, Aug 31, 2018 at 02:20:31PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Aug 31, 2018 at 10:14:14AM +0200, Daniel Vetter wrote:
> > > > > On Thu, Aug 23, 2018 at 06:43:40PM +0100, Alexandru-Cosmin Gheorghe
> > > > > wrote:
> > > > > > On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> > > > > > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe
> > > > > > > wrote:
> > > > > > > > The previous patch added tile_w and tile_h, which represent the
> > > > > > > > horizontal and vertical sizes of a tile.
> > > > > > > >
> > > > > > > > This one uses that to plumb through drm core in order to be
> > > > > > > > able to
> > > > > > > > handle linear tile formats without the need for drivers to roll
> > > > > > > > up
> > > > > > > > their own implementation.
> > > > > > > >
> > > > > > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind
> > > > > > > > which
> > > > > > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in
> > > > > > > > average 2
> > > > > > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > > > > > >
> > > > > > > > Now what are the restrictions:
> > > > > > > >
> > > > > > > > 1. Pitch in bytes is expected to cover at least tile_h * width
> > > > > > > > in
> > > > > > > > pixels. Due to this the places where the pitch is checked/used
> > > > > > > > need to
> > > > > > > > be updated to take into consideration the tile_w, tile_h and
> > > > > > > > tile_size.
> > > > > > > > tile_size = cpp * tile_w * tile_h
> > > > > > > >
> > > > > > > > 2. When doing source cropping plane_src_x/y need to be a
> > > > > > > > multiple of
> > > > > > > > tile_w/tile_h and we need to take into consideration the
> > > > > > > > tile_w/tile_h
> > > > > > > > when computing the start address.
> > > > > > > >
> > > > > > > > For all non-tiled formats the tile_w and tile_h will be 1, so
> > > > > > > > if I
> > > > > > > > didn't miss anything nothing should change.
> > > > > > > >
> > > > > > > > Regarding multi-planar linear tile formats, I'm not sure how
> > > > > > > > those
> > > > > > > > should be handle I kind of assumed that tile_h/tile_w will have
> > > > > > > > to be
> > > > > > > > divided by horizontal/subsampling. Anyway, I think it's best to
> > > > > > > > just
> > > > > > > > put an warning in there and handle it when someone tries to add
> > > > > > > > support for them.
> > > > > > > >
> > > > > > > > Signed-off-by: Alexandru Gheorghe
> > > > > > > >
> > > > > > > > ---
> > > > > > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > > > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > > > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > > > > >
> > > > > > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > > > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > > > > > include/drm/drm_fourcc.h | 2 +
> > > > > > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > > > > > > b/drivers/gpu/drm/drm_atomic.c
> > > > > > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > > > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > > > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > > > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > > > > > > drm_plane *plane,
> > > > > > > > return -ENOSPC;
> > > > > > > > }
> > > > > > > >
> > > > > > > > + /* Make sure source coordinates are a multiple of tile
> > > > > > > > sizes */
> > > > > > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > > > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > > > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source
> > > > > > > > coordinates do not meet tile restrictions",
> > > > > > > > +plane->base.id, plane->name);
> > > > > > > > + return -EINVAL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > if (plane_switching_crtc(state->state, plane, state)) {
> > > > > > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > > > > > > > directly\n",
> > > > > > > > plane->base.id, plane->name);
> > > > > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > > index 47e0e2f6642d..4d8052adce67 100644
> > > > > > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Fri, Aug 31, 2018 at 05:26:37PM +0100, Alexandru-Cosmin Gheorghe wrote:
> Hi,
>
> On Fri, Aug 31, 2018 at 05:12:24PM +0200, Daniel Vetter wrote:
> > On Fri, Aug 31, 2018 at 02:20:31PM +0300, Ville Syrjälä wrote:
> > > On Fri, Aug 31, 2018 at 10:14:14AM +0200, Daniel Vetter wrote:
> > > > On Thu, Aug 23, 2018 at 06:43:40PM +0100, Alexandru-Cosmin Gheorghe
> > > > wrote:
> > > > > On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> > > > > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > > > > > The previous patch added tile_w and tile_h, which represent the
> > > > > > > horizontal and vertical sizes of a tile.
> > > > > > >
> > > > > > > This one uses that to plumb through drm core in order to be able
> > > > > > > to
> > > > > > > handle linear tile formats without the need for drivers to roll up
> > > > > > > their own implementation.
> > > > > > >
> > > > > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind
> > > > > > > which
> > > > > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in
> > > > > > > average 2
> > > > > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > > > > >
> > > > > > > Now what are the restrictions:
> > > > > > >
> > > > > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > > > > > pixels. Due to this the places where the pitch is checked/used
> > > > > > > need to
> > > > > > > be updated to take into consideration the tile_w, tile_h and
> > > > > > > tile_size.
> > > > > > > tile_size = cpp * tile_w * tile_h
> > > > > > >
> > > > > > > 2. When doing source cropping plane_src_x/y need to be a multiple
> > > > > > > of
> > > > > > > tile_w/tile_h and we need to take into consideration the
> > > > > > > tile_w/tile_h
> > > > > > > when computing the start address.
> > > > > > >
> > > > > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > > > > > didn't miss anything nothing should change.
> > > > > > >
> > > > > > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > > > > > should be handle I kind of assumed that tile_h/tile_w will have
> > > > > > > to be
> > > > > > > divided by horizontal/subsampling. Anyway, I think it's best to
> > > > > > > just
> > > > > > > put an warning in there and handle it when someone tries to add
> > > > > > > support for them.
> > > > > > >
> > > > > > > Signed-off-by: Alexandru Gheorghe
> > > > > > >
> > > > > > > ---
> > > > > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > > > >
> > > > > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > > > > include/drm/drm_fourcc.h | 2 +
> > > > > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > > > > > b/drivers/gpu/drm/drm_atomic.c
> > > > > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > > > > > drm_plane *plane,
> > > > > > > return -ENOSPC;
> > > > > > > }
> > > > > > >
> > > > > > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > > > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do
> > > > > > > not meet tile restrictions",
> > > > > > > + plane->base.id, plane->name);
> > > > > > > + return -EINVAL;
> > > > > > > + }
> > > > > > > +
> > > > > > > if (plane_switching_crtc(state->state, plane, state)) {
> > > > > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > > > > > > directly\n",
> > > > > > >plane->base.id, plane->name);
> > > > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > index 47e0e2f6642d..4d8052adce67 100644
> > > > > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > > > drm_framebuffer *fb,
> > > > > > > struct drm_gem_cma_object *obj;
> > > > > > > dma_addr_t paddr;
> > > > > > > u8 h_div = 1, v_div = 1;
> > > > > > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > > > > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > > > > > >
> > > > > > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > > > > > if (!obj)
> > > > > > > @@ -99,8
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Fri, Aug 31, 2018 at 05:12:24PM +0200, Daniel Vetter wrote:
> On Fri, Aug 31, 2018 at 02:20:31PM +0300, Ville Syrjälä wrote:
> > On Fri, Aug 31, 2018 at 10:14:14AM +0200, Daniel Vetter wrote:
> > > On Thu, Aug 23, 2018 at 06:43:40PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > > > On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> > > > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > > > > The previous patch added tile_w and tile_h, which represent the
> > > > > > horizontal and vertical sizes of a tile.
> > > > > >
> > > > > > This one uses that to plumb through drm core in order to be able to
> > > > > > handle linear tile formats without the need for drivers to roll up
> > > > > > their own implementation.
> > > > > >
> > > > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > > > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average
> > > > > > 2
> > > > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > > > >
> > > > > > Now what are the restrictions:
> > > > > >
> > > > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > > > > pixels. Due to this the places where the pitch is checked/used need
> > > > > > to
> > > > > > be updated to take into consideration the tile_w, tile_h and
> > > > > > tile_size.
> > > > > > tile_size = cpp * tile_w * tile_h
> > > > > >
> > > > > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > > > > > tile_w/tile_h and we need to take into consideration the
> > > > > > tile_w/tile_h
> > > > > > when computing the start address.
> > > > > >
> > > > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > > > > didn't miss anything nothing should change.
> > > > > >
> > > > > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > > > > should be handle I kind of assumed that tile_h/tile_w will have to
> > > > > > be
> > > > > > divided by horizontal/subsampling. Anyway, I think it's best to just
> > > > > > put an warning in there and handle it when someone tries to add
> > > > > > support for them.
> > > > > >
> > > > > > Signed-off-by: Alexandru Gheorghe
> > > > > >
> > > > > > ---
> > > > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > > >
> > > > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > > > include/drm/drm_fourcc.h | 2 +
> > > > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > > > > b/drivers/gpu/drm/drm_atomic.c
> > > > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > > > > drm_plane *plane,
> > > > > > return -ENOSPC;
> > > > > > }
> > > > > >
> > > > > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do
> > > > > > not meet tile restrictions",
> > > > > > +plane->base.id, plane->name);
> > > > > > + return -EINVAL;
> > > > > > + }
> > > > > > +
> > > > > > if (plane_switching_crtc(state->state, plane, state)) {
> > > > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > > > > > directly\n",
> > > > > > plane->base.id, plane->name);
> > > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > index 47e0e2f6642d..4d8052adce67 100644
> > > > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > > drm_framebuffer *fb,
> > > > > > struct drm_gem_cma_object *obj;
> > > > > > dma_addr_t paddr;
> > > > > > u8 h_div = 1, v_div = 1;
> > > > > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > > > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > > > > >
> > > > > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > > > > if (!obj)
> > > > > > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > > drm_framebuffer *fb,
> > > > > > v_div = fb->format->vsub;
> > > > > > }
> > > > > >
> > > > > > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) /
> > > > > > h_div;
> > > > > > - paddr +=
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
Hi,
On Fri, Aug 31, 2018 at 05:12:24PM +0200, Daniel Vetter wrote:
> On Fri, Aug 31, 2018 at 02:20:31PM +0300, Ville Syrjälä wrote:
> > On Fri, Aug 31, 2018 at 10:14:14AM +0200, Daniel Vetter wrote:
> > > On Thu, Aug 23, 2018 at 06:43:40PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > > > On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> > > > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > > > > The previous patch added tile_w and tile_h, which represent the
> > > > > > horizontal and vertical sizes of a tile.
> > > > > >
> > > > > > This one uses that to plumb through drm core in order to be able to
> > > > > > handle linear tile formats without the need for drivers to roll up
> > > > > > their own implementation.
> > > > > >
> > > > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > > > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average
> > > > > > 2
> > > > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > > > >
> > > > > > Now what are the restrictions:
> > > > > >
> > > > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > > > > pixels. Due to this the places where the pitch is checked/used need
> > > > > > to
> > > > > > be updated to take into consideration the tile_w, tile_h and
> > > > > > tile_size.
> > > > > > tile_size = cpp * tile_w * tile_h
> > > > > >
> > > > > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > > > > > tile_w/tile_h and we need to take into consideration the
> > > > > > tile_w/tile_h
> > > > > > when computing the start address.
> > > > > >
> > > > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > > > > didn't miss anything nothing should change.
> > > > > >
> > > > > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > > > > should be handle I kind of assumed that tile_h/tile_w will have to
> > > > > > be
> > > > > > divided by horizontal/subsampling. Anyway, I think it's best to just
> > > > > > put an warning in there and handle it when someone tries to add
> > > > > > support for them.
> > > > > >
> > > > > > Signed-off-by: Alexandru Gheorghe
> > > > > >
> > > > > > ---
> > > > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > > >
> > > > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > > > include/drm/drm_fourcc.h | 2 +
> > > > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > > > > b/drivers/gpu/drm/drm_atomic.c
> > > > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > > > > drm_plane *plane,
> > > > > > return -ENOSPC;
> > > > > > }
> > > > > >
> > > > > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do
> > > > > > not meet tile restrictions",
> > > > > > +plane->base.id, plane->name);
> > > > > > + return -EINVAL;
> > > > > > + }
> > > > > > +
> > > > > > if (plane_switching_crtc(state->state, plane, state)) {
> > > > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > > > > > directly\n",
> > > > > > plane->base.id, plane->name);
> > > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > index 47e0e2f6642d..4d8052adce67 100644
> > > > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > > drm_framebuffer *fb,
> > > > > > struct drm_gem_cma_object *obj;
> > > > > > dma_addr_t paddr;
> > > > > > u8 h_div = 1, v_div = 1;
> > > > > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > > > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > > > > >
> > > > > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > > > > if (!obj)
> > > > > > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > > drm_framebuffer *fb,
> > > > > > v_div = fb->format->vsub;
> > > > > > }
> > > > > >
> > > > > > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) /
> > > > > > h_div;
> > > > > > - paddr +=
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Fri, Aug 31, 2018 at 02:20:31PM +0300, Ville Syrjälä wrote:
> On Fri, Aug 31, 2018 at 10:14:14AM +0200, Daniel Vetter wrote:
> > On Thu, Aug 23, 2018 at 06:43:40PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > > On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> > > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > > > The previous patch added tile_w and tile_h, which represent the
> > > > > horizontal and vertical sizes of a tile.
> > > > >
> > > > > This one uses that to plumb through drm core in order to be able to
> > > > > handle linear tile formats without the need for drivers to roll up
> > > > > their own implementation.
> > > > >
> > > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> > > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > > >
> > > > > Now what are the restrictions:
> > > > >
> > > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > > > pixels. Due to this the places where the pitch is checked/used need to
> > > > > be updated to take into consideration the tile_w, tile_h and
> > > > > tile_size.
> > > > > tile_size = cpp * tile_w * tile_h
> > > > >
> > > > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > > > > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> > > > > when computing the start address.
> > > > >
> > > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > > > didn't miss anything nothing should change.
> > > > >
> > > > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > > > should be handle I kind of assumed that tile_h/tile_w will have to be
> > > > > divided by horizontal/subsampling. Anyway, I think it's best to just
> > > > > put an warning in there and handle it when someone tries to add
> > > > > support for them.
> > > > >
> > > > > Signed-off-by: Alexandru Gheorghe
> > > > > ---
> > > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > >
> > > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > > include/drm/drm_fourcc.h | 2 +
> > > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > > > b/drivers/gpu/drm/drm_atomic.c
> > > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > > > drm_plane *plane,
> > > > > return -ENOSPC;
> > > > > }
> > > > >
> > > > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do
> > > > > not meet tile restrictions",
> > > > > + plane->base.id, plane->name);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > if (plane_switching_crtc(state->state, plane, state)) {
> > > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > > > > directly\n",
> > > > >plane->base.id, plane->name);
> > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > index 47e0e2f6642d..4d8052adce67 100644
> > > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > drm_framebuffer *fb,
> > > > > struct drm_gem_cma_object *obj;
> > > > > dma_addr_t paddr;
> > > > > u8 h_div = 1, v_div = 1;
> > > > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > > > >
> > > > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > > > if (!obj)
> > > > > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > > drm_framebuffer *fb,
> > > > > v_div = fb->format->vsub;
> > > > > }
> > > > >
> > > > > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) /
> > > > > h_div;
> > > > > - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> > > > > + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >>
> > > > > 16))
> > > > > + / h_div;
> > > > > + /*
> > > > > + * For tile formats pitches are expected to cover at least
> > > >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Fri, Aug 31, 2018 at 10:14:14AM +0200, Daniel Vetter wrote:
> On Thu, Aug 23, 2018 at 06:43:40PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > > The previous patch added tile_w and tile_h, which represent the
> > > > horizontal and vertical sizes of a tile.
> > > >
> > > > This one uses that to plumb through drm core in order to be able to
> > > > handle linear tile formats without the need for drivers to roll up
> > > > their own implementation.
> > > >
> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > >
> > > > Now what are the restrictions:
> > > >
> > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > > pixels. Due to this the places where the pitch is checked/used need to
> > > > be updated to take into consideration the tile_w, tile_h and
> > > > tile_size.
> > > > tile_size = cpp * tile_w * tile_h
> > > >
> > > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > > > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> > > > when computing the start address.
> > > >
> > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > > didn't miss anything nothing should change.
> > > >
> > > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > > should be handle I kind of assumed that tile_h/tile_w will have to be
> > > > divided by horizontal/subsampling. Anyway, I think it's best to just
> > > > put an warning in there and handle it when someone tries to add
> > > > support for them.
> > > >
> > > > Signed-off-by: Alexandru Gheorghe
> > > > ---
> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > include/drm/drm_fourcc.h | 2 +
> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > > drm_plane *plane,
> > > > return -ENOSPC;
> > > > }
> > > >
> > > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do
> > > > not meet tile restrictions",
> > > > +plane->base.id, plane->name);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > if (plane_switching_crtc(state->state, plane, state)) {
> > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > > > directly\n",
> > > > plane->base.id, plane->name);
> > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > index 47e0e2f6642d..4d8052adce67 100644
> > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > drm_framebuffer *fb,
> > > > struct drm_gem_cma_object *obj;
> > > > dma_addr_t paddr;
> > > > u8 h_div = 1, v_div = 1;
> > > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > > >
> > > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > > if (!obj)
> > > > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > drm_framebuffer *fb,
> > > > v_div = fb->format->vsub;
> > > > }
> > > >
> > > > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) /
> > > > h_div;
> > > > - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> > > > + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >>
> > > > 16))
> > > > + / h_div;
> > > > + /*
> > > > +* For tile formats pitches are expected to cover at least
> > > > +* width * tile_h pixels
> > > > +*/
> > > > + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16))
> > > > / v_div;
> > > >
> > > > return paddr;
> > > > }
> > > > diff --git
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Thu, Aug 23, 2018 at 06:43:40PM +0100, Alexandru-Cosmin Gheorghe wrote:
> On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > The previous patch added tile_w and tile_h, which represent the
> > > horizontal and vertical sizes of a tile.
> > >
> > > This one uses that to plumb through drm core in order to be able to
> > > handle linear tile formats without the need for drivers to roll up
> > > their own implementation.
> > >
> > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> > > bytes per pixel and where tiles are laid out in a linear manner.
> > >
> > > Now what are the restrictions:
> > >
> > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > pixels. Due to this the places where the pitch is checked/used need to
> > > be updated to take into consideration the tile_w, tile_h and
> > > tile_size.
> > > tile_size = cpp * tile_w * tile_h
> > >
> > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> > > when computing the start address.
> > >
> > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > didn't miss anything nothing should change.
> > >
> > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > should be handle I kind of assumed that tile_h/tile_w will have to be
> > > divided by horizontal/subsampling. Anyway, I think it's best to just
> > > put an warning in there and handle it when someone tries to add
> > > support for them.
> > >
> > > Signed-off-by: Alexandru Gheorghe
> > > ---
> > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > drivers/gpu/drm/drm_fourcc.c | 52
> > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > include/drm/drm_fourcc.h | 2 +
> > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> > > *plane,
> > > return -ENOSPC;
> > > }
> > >
> > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> > > tile restrictions",
> > > + plane->base.id, plane->name);
> > > + return -EINVAL;
> > > + }
> > > +
> > > if (plane_switching_crtc(state->state, plane, state)) {
> > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> > >plane->base.id, plane->name);
> > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > index 47e0e2f6642d..4d8052adce67 100644
> > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > drm_framebuffer *fb,
> > > struct drm_gem_cma_object *obj;
> > > dma_addr_t paddr;
> > > u8 h_div = 1, v_div = 1;
> > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > >
> > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > if (!obj)
> > > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > drm_framebuffer *fb,
> > > v_div = fb->format->vsub;
> > > }
> > >
> > > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> > > - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> > > + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> > > + / h_div;
> > > + /*
> > > + * For tile formats pitches are expected to cover at least
> > > + * width * tile_h pixels
> > > + */
> > > + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
> > >
> > > return paddr;
> > > }
> > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > > index f55cd93ba2d0..d6c9c5aa4036 100644
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t
> > > format, int plane)
> > > return height / info->vsub;
> > > }
> > > EXPORT_SYMBOL(drm_format_plane_height);
> > > +
> > > +/**
> > > + * drm_format_tile_width - width of a tile for tile formats, should be 1
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Thu, Aug 23, 2018 at 06:19:31PM +0100, Alexandru-Cosmin Gheorghe wrote:
> On Thu, Aug 23, 2018 at 05:30:36PM +0300, Ville Syrjälä wrote:
> > On Wed, Aug 22, 2018 at 09:48:29PM +0200, Daniel Vetter wrote:
> > > On Wed, Aug 22, 2018 at 4:05 PM, Alexandru-Cosmin Gheorghe
> > > wrote:
> > > > On Wed, Aug 22, 2018 at 04:45:34PM +0300, Ville Syrjälä wrote:
> > > >> On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe
> > > >> wrote:
> > > >> > On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> > > >> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > >> > > > The previous patch added tile_w and tile_h, which represent the
> > > >> > > > horizontal and vertical sizes of a tile.
> > > >> > > >
> > > >> > > > This one uses that to plumb through drm core in order to be able
> > > >> > > > to
> > > >> > > > handle linear tile formats without the need for drivers to roll
> > > >> > > > up
> > > >> > > > their own implementation.
> > > >> > > >
> > > >> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind
> > > >> > > > which
> > > >> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in
> > > >> > > > average 2
> > > >> > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > >> > > >
> > > >> > > > Now what are the restrictions:
> > > >> > > >
> > > >> > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > >> > > > pixels. Due to this the places where the pitch is checked/used
> > > >> > > > need to
> > > >> > > > be updated to take into consideration the tile_w, tile_h and
> > > >> > > > tile_size.
> > > >> > > > tile_size = cpp * tile_w * tile_h
> > > >> > > >
> > > >> > > > 2. When doing source cropping plane_src_x/y need to be a
> > > >> > > > multiple of
> > > >> > > > tile_w/tile_h and we need to take into consideration the
> > > >> > > > tile_w/tile_h
> > > >> > > > when computing the start address.
> > > >> > > >
> > > >> > > > For all non-tiled formats the tile_w and tile_h will be 1, so if
> > > >> > > > I
> > > >> > > > didn't miss anything nothing should change.
> > > >> > > >
> > > >> > > > Regarding multi-planar linear tile formats, I'm not sure how
> > > >> > > > those
> > > >> > > > should be handle I kind of assumed that tile_h/tile_w will have
> > > >> > > > to be
> > > >> > > > divided by horizontal/subsampling. Anyway, I think it's best to
> > > >> > > > just
> > > >> > > > put an warning in there and handle it when someone tries to add
> > > >> > > > support for them.
> > > >> > > >
> > > >> > > > Signed-off-by: Alexandru Gheorghe
> > > >> > > >
> > > >> > > > ---
> > > >> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > >> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > >> > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > >> > > >
> > > >> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > >> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > >> > > > include/drm/drm_fourcc.h | 2 +
> > > >> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > >> > > >
> > > >> > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > >> > > > b/drivers/gpu/drm/drm_atomic.c
> > > >> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > >> > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > >> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > >> > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > >> > > > drm_plane *plane,
> > > >> > > > return -ENOSPC;
> > > >> > > > }
> > > >> > > >
> > > >> > > > + /* Make sure source coordinates are a multiple of tile
> > > >> > > > sizes */
> > > >> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > >> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > >> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source
> > > >> > > > coordinates do not meet tile restrictions",
> > > >> > > > +plane->base.id, plane->name);
> > > >> > > > + return -EINVAL;
> > > >> > > > + }
> > > >> > >
> > > >> > > Feels rather wrong to me to put such hardware specific limitations
> > > >> > > into
> > > >> > > the core.
> > > >> >
> > > >> > Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
> > > >> > work.
> > > >>
> > > >> If that guy is supposed to give you a tile aligned address then it
> > > >> could
> > > >> just do that and leave it up to the driver to deal with the remainder
> > > >> of
> > > >> the coordinates?
> > > >>
> > > >> >
> > > >> > An alternative, would be to include it in the driver and put an WARN
> > > >> > in drm_fb_cma_get_gem_addr in case someone else uses it with a
> > > >> > src_x/src_y tile multiples.
> > > >> >
> > > >> > What do you think about that ?
> > > >> >
> > > >> > >
> > > >> > > > +
> > > >> > > >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Thu, Aug 23, 2018 at 08:25:46PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 23, 2018 at 06:19:31PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > On Thu, Aug 23, 2018 at 05:30:36PM +0300, Ville Syrjälä wrote:
> > > On Wed, Aug 22, 2018 at 09:48:29PM +0200, Daniel Vetter wrote:
> > > > On Wed, Aug 22, 2018 at 4:05 PM, Alexandru-Cosmin Gheorghe
> > > > wrote:
> > > > > On Wed, Aug 22, 2018 at 04:45:34PM +0300, Ville Syrjälä wrote:
> > > > >> On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe
> > > > >> wrote:
> > > > >> > On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> > > > >> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe
> > > > >> > > wrote:
> > > > >> > > > The previous patch added tile_w and tile_h, which represent the
> > > > >> > > > horizontal and vertical sizes of a tile.
> > > > >> > > >
> > > > >> > > > This one uses that to plumb through drm core in order to be
> > > > >> > > > able to
> > > > >> > > > handle linear tile formats without the need for drivers to
> > > > >> > > > roll up
> > > > >> > > > their own implementation.
> > > > >> > > >
> > > > >> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind
> > > > >> > > > which
> > > > >> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in
> > > > >> > > > average 2
> > > > >> > > > bytes per pixel and where tiles are laid out in a linear
> > > > >> > > > manner.
> > > > >> > > >
> > > > >> > > > Now what are the restrictions:
> > > > >> > > >
> > > > >> > > > 1. Pitch in bytes is expected to cover at least tile_h * width
> > > > >> > > > in
> > > > >> > > > pixels. Due to this the places where the pitch is checked/used
> > > > >> > > > need to
> > > > >> > > > be updated to take into consideration the tile_w, tile_h and
> > > > >> > > > tile_size.
> > > > >> > > > tile_size = cpp * tile_w * tile_h
> > > > >> > > >
> > > > >> > > > 2. When doing source cropping plane_src_x/y need to be a
> > > > >> > > > multiple of
> > > > >> > > > tile_w/tile_h and we need to take into consideration the
> > > > >> > > > tile_w/tile_h
> > > > >> > > > when computing the start address.
> > > > >> > > >
> > > > >> > > > For all non-tiled formats the tile_w and tile_h will be 1, so
> > > > >> > > > if I
> > > > >> > > > didn't miss anything nothing should change.
> > > > >> > > >
> > > > >> > > > Regarding multi-planar linear tile formats, I'm not sure how
> > > > >> > > > those
> > > > >> > > > should be handle I kind of assumed that tile_h/tile_w will
> > > > >> > > > have to be
> > > > >> > > > divided by horizontal/subsampling. Anyway, I think it's best
> > > > >> > > > to just
> > > > >> > > > put an warning in there and handle it when someone tries to add
> > > > >> > > > support for them.
> > > > >> > > >
> > > > >> > > > Signed-off-by: Alexandru Gheorghe
> > > > >> > > >
> > > > >> > > > ---
> > > > >> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > >> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > >> > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > >> > > >
> > > > >> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > >> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > >> > > > include/drm/drm_fourcc.h | 2 +
> > > > >> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > > >> > > >
> > > > >> > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > > >> > > > b/drivers/gpu/drm/drm_atomic.c
> > > > >> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > >> > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > >> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > >> > > > @@ -1087,6 +1087,14 @@ static int
> > > > >> > > > drm_atomic_plane_check(struct drm_plane *plane,
> > > > >> > > > return -ENOSPC;
> > > > >> > > > }
> > > > >> > > >
> > > > >> > > > + /* Make sure source coordinates are a multiple of tile
> > > > >> > > > sizes */
> > > > >> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > >> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > >> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source
> > > > >> > > > coordinates do not meet tile restrictions",
> > > > >> > > > +plane->base.id, plane->name);
> > > > >> > > > + return -EINVAL;
> > > > >> > > > + }
> > > > >> > >
> > > > >> > > Feels rather wrong to me to put such hardware specific
> > > > >> > > limitations into
> > > > >> > > the core.
> > > > >> >
> > > > >> > Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr
> > > > >> > wouldn't
> > > > >> > work.
> > > > >>
> > > > >> If that guy is supposed to give you a tile aligned address then it
> > > > >> could
> > > > >> just do that and leave it up to the driver to deal with the
> > > > >> remainder of
> > > > >> the coordinates?
> > >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Wed, Aug 22, 2018 at 10:18:51PM +0200, Daniel Vetter wrote:
> On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > The previous patch added tile_w and tile_h, which represent the
> > horizontal and vertical sizes of a tile.
> >
> > This one uses that to plumb through drm core in order to be able to
> > handle linear tile formats without the need for drivers to roll up
> > their own implementation.
> >
> > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> > bytes per pixel and where tiles are laid out in a linear manner.
> >
> > Now what are the restrictions:
> >
> > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > pixels. Due to this the places where the pitch is checked/used need to
> > be updated to take into consideration the tile_w, tile_h and
> > tile_size.
> > tile_size = cpp * tile_w * tile_h
> >
> > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> > when computing the start address.
> >
> > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > didn't miss anything nothing should change.
> >
> > Regarding multi-planar linear tile formats, I'm not sure how those
> > should be handle I kind of assumed that tile_h/tile_w will have to be
> > divided by horizontal/subsampling. Anyway, I think it's best to just
> > put an warning in there and handle it when someone tries to add
> > support for them.
> >
> > Signed-off-by: Alexandru Gheorghe
> > ---
> > drivers/gpu/drm/drm_atomic.c | 8 +++
> > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > drivers/gpu/drm/drm_fourcc.c | 52
> > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > include/drm/drm_fourcc.h | 2 +
> > 6 files changed, 94 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 3eb061e11e2e..7a3e893a4cd1 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> > *plane,
> > return -ENOSPC;
> > }
> >
> > + /* Make sure source coordinates are a multiple of tile sizes */
> > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> > tile restrictions",
> > +plane->base.id, plane->name);
> > + return -EINVAL;
> > + }
> > +
> > if (plane_switching_crtc(state->state, plane, state)) {
> > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> > plane->base.id, plane->name);
> > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > index 47e0e2f6642d..4d8052adce67 100644
> > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> > *fb,
> > struct drm_gem_cma_object *obj;
> > dma_addr_t paddr;
> > u8 h_div = 1, v_div = 1;
> > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> >
> > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > if (!obj)
> > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > drm_framebuffer *fb,
> > v_div = fb->format->vsub;
> > }
> >
> > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> > - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> > + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> > + / h_div;
> > + /*
> > +* For tile formats pitches are expected to cover at least
> > +* width * tile_h pixels
> > +*/
> > + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
> >
> > return paddr;
> > }
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index f55cd93ba2d0..d6c9c5aa4036 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t
> > format, int plane)
> > return height / info->vsub;
> > }
> > EXPORT_SYMBOL(drm_format_plane_height);
> > +
> > +/**
> > + * drm_format_tile_width - width of a tile for tile formats, should be 1
> > for all
> > + * non-tiled formats.
> > + * @format: pixel format
> > + * @plane: plane index
> > + *
> > + * Returns:
> > + * The width of a tile, depending on the plane index and horizontal
> > sub-sampling
> > + */
> >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Thu, Aug 23, 2018 at 06:19:31PM +0100, Alexandru-Cosmin Gheorghe wrote:
> On Thu, Aug 23, 2018 at 05:30:36PM +0300, Ville Syrjälä wrote:
> > On Wed, Aug 22, 2018 at 09:48:29PM +0200, Daniel Vetter wrote:
> > > On Wed, Aug 22, 2018 at 4:05 PM, Alexandru-Cosmin Gheorghe
> > > wrote:
> > > > On Wed, Aug 22, 2018 at 04:45:34PM +0300, Ville Syrjälä wrote:
> > > >> On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe
> > > >> wrote:
> > > >> > On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> > > >> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > >> > > > The previous patch added tile_w and tile_h, which represent the
> > > >> > > > horizontal and vertical sizes of a tile.
> > > >> > > >
> > > >> > > > This one uses that to plumb through drm core in order to be able
> > > >> > > > to
> > > >> > > > handle linear tile formats without the need for drivers to roll
> > > >> > > > up
> > > >> > > > their own implementation.
> > > >> > > >
> > > >> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind
> > > >> > > > which
> > > >> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in
> > > >> > > > average 2
> > > >> > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > >> > > >
> > > >> > > > Now what are the restrictions:
> > > >> > > >
> > > >> > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > >> > > > pixels. Due to this the places where the pitch is checked/used
> > > >> > > > need to
> > > >> > > > be updated to take into consideration the tile_w, tile_h and
> > > >> > > > tile_size.
> > > >> > > > tile_size = cpp * tile_w * tile_h
> > > >> > > >
> > > >> > > > 2. When doing source cropping plane_src_x/y need to be a
> > > >> > > > multiple of
> > > >> > > > tile_w/tile_h and we need to take into consideration the
> > > >> > > > tile_w/tile_h
> > > >> > > > when computing the start address.
> > > >> > > >
> > > >> > > > For all non-tiled formats the tile_w and tile_h will be 1, so if
> > > >> > > > I
> > > >> > > > didn't miss anything nothing should change.
> > > >> > > >
> > > >> > > > Regarding multi-planar linear tile formats, I'm not sure how
> > > >> > > > those
> > > >> > > > should be handle I kind of assumed that tile_h/tile_w will have
> > > >> > > > to be
> > > >> > > > divided by horizontal/subsampling. Anyway, I think it's best to
> > > >> > > > just
> > > >> > > > put an warning in there and handle it when someone tries to add
> > > >> > > > support for them.
> > > >> > > >
> > > >> > > > Signed-off-by: Alexandru Gheorghe
> > > >> > > >
> > > >> > > > ---
> > > >> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > >> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > >> > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > >> > > >
> > > >> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > >> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > >> > > > include/drm/drm_fourcc.h | 2 +
> > > >> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > >> > > >
> > > >> > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > > >> > > > b/drivers/gpu/drm/drm_atomic.c
> > > >> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > >> > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > >> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > >> > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > >> > > > drm_plane *plane,
> > > >> > > > return -ENOSPC;
> > > >> > > > }
> > > >> > > >
> > > >> > > > + /* Make sure source coordinates are a multiple of tile
> > > >> > > > sizes */
> > > >> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > >> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > >> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source
> > > >> > > > coordinates do not meet tile restrictions",
> > > >> > > > +plane->base.id, plane->name);
> > > >> > > > + return -EINVAL;
> > > >> > > > + }
> > > >> > >
> > > >> > > Feels rather wrong to me to put such hardware specific limitations
> > > >> > > into
> > > >> > > the core.
> > > >> >
> > > >> > Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
> > > >> > work.
> > > >>
> > > >> If that guy is supposed to give you a tile aligned address then it
> > > >> could
> > > >> just do that and leave it up to the driver to deal with the remainder
> > > >> of
> > > >> the coordinates?
> > > >>
> > > >> >
> > > >> > An alternative, would be to include it in the driver and put an WARN
> > > >> > in drm_fb_cma_get_gem_addr in case someone else uses it with a
> > > >> > src_x/src_y tile multiples.
> > > >> >
> > > >> > What do you think about that ?
> > > >> >
> > > >> > >
> > > >> > > > +
> > > >> > > >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Thu, Aug 23, 2018 at 05:30:36PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 22, 2018 at 09:48:29PM +0200, Daniel Vetter wrote:
> > On Wed, Aug 22, 2018 at 4:05 PM, Alexandru-Cosmin Gheorghe
> > wrote:
> > > On Wed, Aug 22, 2018 at 04:45:34PM +0300, Ville Syrjälä wrote:
> > >> On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe
> > >> wrote:
> > >> > On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> > >> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > >> > > > The previous patch added tile_w and tile_h, which represent the
> > >> > > > horizontal and vertical sizes of a tile.
> > >> > > >
> > >> > > > This one uses that to plumb through drm core in order to be able to
> > >> > > > handle linear tile formats without the need for drivers to roll up
> > >> > > > their own implementation.
> > >> > > >
> > >> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind
> > >> > > > which
> > >> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in
> > >> > > > average 2
> > >> > > > bytes per pixel and where tiles are laid out in a linear manner.
> > >> > > >
> > >> > > > Now what are the restrictions:
> > >> > > >
> > >> > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > >> > > > pixels. Due to this the places where the pitch is checked/used
> > >> > > > need to
> > >> > > > be updated to take into consideration the tile_w, tile_h and
> > >> > > > tile_size.
> > >> > > > tile_size = cpp * tile_w * tile_h
> > >> > > >
> > >> > > > 2. When doing source cropping plane_src_x/y need to be a multiple
> > >> > > > of
> > >> > > > tile_w/tile_h and we need to take into consideration the
> > >> > > > tile_w/tile_h
> > >> > > > when computing the start address.
> > >> > > >
> > >> > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > >> > > > didn't miss anything nothing should change.
> > >> > > >
> > >> > > > Regarding multi-planar linear tile formats, I'm not sure how those
> > >> > > > should be handle I kind of assumed that tile_h/tile_w will have to
> > >> > > > be
> > >> > > > divided by horizontal/subsampling. Anyway, I think it's best to
> > >> > > > just
> > >> > > > put an warning in there and handle it when someone tries to add
> > >> > > > support for them.
> > >> > > >
> > >> > > > Signed-off-by: Alexandru Gheorghe
> > >> > > >
> > >> > > > ---
> > >> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > >> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > >> > > > drivers/gpu/drm/drm_fourcc.c | 52
> > >> > > >
> > >> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > >> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > >> > > > include/drm/drm_fourcc.h | 2 +
> > >> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > >> > > >
> > >> > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> > >> > > > b/drivers/gpu/drm/drm_atomic.c
> > >> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > >> > > > --- a/drivers/gpu/drm/drm_atomic.c
> > >> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > >> > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > >> > > > drm_plane *plane,
> > >> > > > return -ENOSPC;
> > >> > > > }
> > >> > > >
> > >> > > > + /* Make sure source coordinates are a multiple of tile
> > >> > > > sizes */
> > >> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > >> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > >> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates
> > >> > > > do not meet tile restrictions",
> > >> > > > +plane->base.id, plane->name);
> > >> > > > + return -EINVAL;
> > >> > > > + }
> > >> > >
> > >> > > Feels rather wrong to me to put such hardware specific limitations
> > >> > > into
> > >> > > the core.
> > >> >
> > >> > Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
> > >> > work.
> > >>
> > >> If that guy is supposed to give you a tile aligned address then it could
> > >> just do that and leave it up to the driver to deal with the remainder of
> > >> the coordinates?
> > >>
> > >> >
> > >> > An alternative, would be to include it in the driver and put an WARN
> > >> > in drm_fb_cma_get_gem_addr in case someone else uses it with a
> > >> > src_x/src_y tile multiples.
> > >> >
> > >> > What do you think about that ?
> > >> >
> > >> > >
> > >> > > > +
> > >> > > > if (plane_switching_crtc(state->state, plane, state)) {
> > >> > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > >> > > > directly\n",
> > >> > > > plane->base.id, plane->name);
> > >> > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > >> > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > >> > >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Wed, Aug 22, 2018 at 09:48:29PM +0200, Daniel Vetter wrote:
> On Wed, Aug 22, 2018 at 4:05 PM, Alexandru-Cosmin Gheorghe
> wrote:
> > On Wed, Aug 22, 2018 at 04:45:34PM +0300, Ville Syrjälä wrote:
> >> On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe wrote:
> >> > On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> >> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> >> > > > The previous patch added tile_w and tile_h, which represent the
> >> > > > horizontal and vertical sizes of a tile.
> >> > > >
> >> > > > This one uses that to plumb through drm core in order to be able to
> >> > > > handle linear tile formats without the need for drivers to roll up
> >> > > > their own implementation.
> >> > > >
> >> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> >> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> >> > > > bytes per pixel and where tiles are laid out in a linear manner.
> >> > > >
> >> > > > Now what are the restrictions:
> >> > > >
> >> > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> >> > > > pixels. Due to this the places where the pitch is checked/used need
> >> > > > to
> >> > > > be updated to take into consideration the tile_w, tile_h and
> >> > > > tile_size.
> >> > > > tile_size = cpp * tile_w * tile_h
> >> > > >
> >> > > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> >> > > > tile_w/tile_h and we need to take into consideration the
> >> > > > tile_w/tile_h
> >> > > > when computing the start address.
> >> > > >
> >> > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> >> > > > didn't miss anything nothing should change.
> >> > > >
> >> > > > Regarding multi-planar linear tile formats, I'm not sure how those
> >> > > > should be handle I kind of assumed that tile_h/tile_w will have to be
> >> > > > divided by horizontal/subsampling. Anyway, I think it's best to just
> >> > > > put an warning in there and handle it when someone tries to add
> >> > > > support for them.
> >> > > >
> >> > > > Signed-off-by: Alexandru Gheorghe
> >> > > > ---
> >> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> >> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> >> > > > drivers/gpu/drm/drm_fourcc.c | 52
> >> > > >
> >> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> >> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> >> > > > include/drm/drm_fourcc.h | 2 +
> >> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> >> > > >
> >> > > > diff --git a/drivers/gpu/drm/drm_atomic.c
> >> > > > b/drivers/gpu/drm/drm_atomic.c
> >> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> >> > > > --- a/drivers/gpu/drm/drm_atomic.c
> >> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> >> > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> >> > > > drm_plane *plane,
> >> > > > return -ENOSPC;
> >> > > > }
> >> > > >
> >> > > > + /* Make sure source coordinates are a multiple of tile sizes
> >> > > > */
> >> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> >> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> >> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates
> >> > > > do not meet tile restrictions",
> >> > > > +plane->base.id, plane->name);
> >> > > > + return -EINVAL;
> >> > > > + }
> >> > >
> >> > > Feels rather wrong to me to put such hardware specific limitations into
> >> > > the core.
> >> >
> >> > Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
> >> > work.
> >>
> >> If that guy is supposed to give you a tile aligned address then it could
> >> just do that and leave it up to the driver to deal with the remainder of
> >> the coordinates?
> >>
> >> >
> >> > An alternative, would be to include it in the driver and put an WARN
> >> > in drm_fb_cma_get_gem_addr in case someone else uses it with a
> >> > src_x/src_y tile multiples.
> >> >
> >> > What do you think about that ?
> >> >
> >> > >
> >> > > > +
> >> > > > if (plane_switching_crtc(state->state, plane, state)) {
> >> > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> >> > > > directly\n",
> >> > > > plane->base.id, plane->name);
> >> > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> >> > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> >> > > > index 47e0e2f6642d..4d8052adce67 100644
> >> > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> >> > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> >> > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> >> > > > drm_framebuffer *fb,
> >> > > > struct drm_gem_cma_object *obj;
> >> > > > dma_addr_t paddr;
> >> > > > u8
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> The previous patch added tile_w and tile_h, which represent the
> horizontal and vertical sizes of a tile.
>
> This one uses that to plumb through drm core in order to be able to
> handle linear tile formats without the need for drivers to roll up
> their own implementation.
>
> This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> bytes per pixel and where tiles are laid out in a linear manner.
>
> Now what are the restrictions:
>
> 1. Pitch in bytes is expected to cover at least tile_h * width in
> pixels. Due to this the places where the pitch is checked/used need to
> be updated to take into consideration the tile_w, tile_h and
> tile_size.
> tile_size = cpp * tile_w * tile_h
>
> 2. When doing source cropping plane_src_x/y need to be a multiple of
> tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> when computing the start address.
>
> For all non-tiled formats the tile_w and tile_h will be 1, so if I
> didn't miss anything nothing should change.
>
> Regarding multi-planar linear tile formats, I'm not sure how those
> should be handle I kind of assumed that tile_h/tile_w will have to be
> divided by horizontal/subsampling. Anyway, I think it's best to just
> put an warning in there and handle it when someone tries to add
> support for them.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drivers/gpu/drm/drm_atomic.c | 8 +++
> drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> drivers/gpu/drm/drm_fourcc.c | 52
> drivers/gpu/drm/drm_framebuffer.c| 19 +--
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> include/drm/drm_fourcc.h | 2 +
> 6 files changed, 94 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3eb061e11e2e..7a3e893a4cd1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> *plane,
> return -ENOSPC;
> }
>
> + /* Make sure source coordinates are a multiple of tile sizes */
> + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> + (state->src_y >> 16) % state->fb->format->tile_h) {
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> tile restrictions",
> + plane->base.id, plane->name);
> + return -EINVAL;
> + }
> +
> if (plane_switching_crtc(state->state, plane, state)) {
> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
>plane->base.id, plane->name);
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 47e0e2f6642d..4d8052adce67 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> struct drm_gem_cma_object *obj;
> dma_addr_t paddr;
> u8 h_div = 1, v_div = 1;
> + u32 tile_w = drm_format_tile_width(fb->format, plane);
> + u32 tile_h = drm_format_tile_height(fb->format, plane);
>
> obj = drm_fb_cma_get_gem_obj(fb, plane);
> if (!obj)
> @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> v_div = fb->format->vsub;
> }
>
> - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> + / h_div;
> + /*
> + * For tile formats pitches are expected to cover at least
> + * width * tile_h pixels
> + */
> + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
>
> return paddr;
> }
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index f55cd93ba2d0..d6c9c5aa4036 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t format,
> int plane)
> return height / info->vsub;
> }
> EXPORT_SYMBOL(drm_format_plane_height);
> +
> +/**
> + * drm_format_tile_width - width of a tile for tile formats, should be 1 for
> all
> + * non-tiled formats.
> + * @format: pixel format
> + * @plane: plane index
> + *
> + * Returns:
> + * The width of a tile, depending on the plane index and horizontal
> sub-sampling
> + */
> +uint32_t drm_format_tile_width(const struct drm_format_info *info, int plane)
> +{
> + WARN_ON(!info->tile_w);
> + if (plane == 0 || info->tile_w == 1)
> + return info->tile_w;
> +
> + /*
> + * Multi planar
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> The previous patch added tile_w and tile_h, which represent the
> horizontal and vertical sizes of a tile.
>
> This one uses that to plumb through drm core in order to be able to
> handle linear tile formats without the need for drivers to roll up
> their own implementation.
>
> This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> bytes per pixel and where tiles are laid out in a linear manner.
>
> Now what are the restrictions:
>
> 1. Pitch in bytes is expected to cover at least tile_h * width in
> pixels. Due to this the places where the pitch is checked/used need to
> be updated to take into consideration the tile_w, tile_h and
> tile_size.
> tile_size = cpp * tile_w * tile_h
>
> 2. When doing source cropping plane_src_x/y need to be a multiple of
> tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> when computing the start address.
>
> For all non-tiled formats the tile_w and tile_h will be 1, so if I
> didn't miss anything nothing should change.
>
> Regarding multi-planar linear tile formats, I'm not sure how those
> should be handle I kind of assumed that tile_h/tile_w will have to be
> divided by horizontal/subsampling. Anyway, I think it's best to just
> put an warning in there and handle it when someone tries to add
> support for them.
>
> Signed-off-by: Alexandru Gheorghe
Just general comment: I think it'd be awesome (and also a great
demonstration that the tile_h/w approach is sound) if we could subsume the
DRM_FORMAT_MOD_SAMSUNG_64_32_TILE stuff into this.
Probably need a core change to drm_get_format_info for this pair (and
reject it for everything else), with the correct tile sizes set.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 8 +++
> drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> drivers/gpu/drm/drm_fourcc.c | 52
> drivers/gpu/drm/drm_framebuffer.c| 19 +--
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> include/drm/drm_fourcc.h | 2 +
> 6 files changed, 94 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3eb061e11e2e..7a3e893a4cd1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> *plane,
> return -ENOSPC;
> }
>
> + /* Make sure source coordinates are a multiple of tile sizes */
> + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> + (state->src_y >> 16) % state->fb->format->tile_h) {
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> tile restrictions",
> + plane->base.id, plane->name);
> + return -EINVAL;
> + }
> +
> if (plane_switching_crtc(state->state, plane, state)) {
> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
>plane->base.id, plane->name);
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 47e0e2f6642d..4d8052adce67 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> struct drm_gem_cma_object *obj;
> dma_addr_t paddr;
> u8 h_div = 1, v_div = 1;
> + u32 tile_w = drm_format_tile_width(fb->format, plane);
> + u32 tile_h = drm_format_tile_height(fb->format, plane);
>
> obj = drm_fb_cma_get_gem_obj(fb, plane);
> if (!obj)
> @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> v_div = fb->format->vsub;
> }
>
> - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> + / h_div;
> + /*
> + * For tile formats pitches are expected to cover at least
> + * width * tile_h pixels
> + */
> + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
>
> return paddr;
> }
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index f55cd93ba2d0..d6c9c5aa4036 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t format,
> int plane)
> return height / info->vsub;
> }
> EXPORT_SYMBOL(drm_format_plane_height);
> +
> +/**
> + * drm_format_tile_width - width of a tile for tile formats, should be 1 for
> all
> + * non-tiled formats.
> + * @format: pixel format
> + * @plane: plane index
> + *
> + *
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Wed, Aug 22, 2018 at 4:05 PM, Alexandru-Cosmin Gheorghe
wrote:
> On Wed, Aug 22, 2018 at 04:45:34PM +0300, Ville Syrjälä wrote:
>> On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe wrote:
>> > On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
>> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
>> > > > The previous patch added tile_w and tile_h, which represent the
>> > > > horizontal and vertical sizes of a tile.
>> > > >
>> > > > This one uses that to plumb through drm core in order to be able to
>> > > > handle linear tile formats without the need for drivers to roll up
>> > > > their own implementation.
>> > > >
>> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
>> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
>> > > > bytes per pixel and where tiles are laid out in a linear manner.
>> > > >
>> > > > Now what are the restrictions:
>> > > >
>> > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
>> > > > pixels. Due to this the places where the pitch is checked/used need to
>> > > > be updated to take into consideration the tile_w, tile_h and
>> > > > tile_size.
>> > > > tile_size = cpp * tile_w * tile_h
>> > > >
>> > > > 2. When doing source cropping plane_src_x/y need to be a multiple of
>> > > > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
>> > > > when computing the start address.
>> > > >
>> > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
>> > > > didn't miss anything nothing should change.
>> > > >
>> > > > Regarding multi-planar linear tile formats, I'm not sure how those
>> > > > should be handle I kind of assumed that tile_h/tile_w will have to be
>> > > > divided by horizontal/subsampling. Anyway, I think it's best to just
>> > > > put an warning in there and handle it when someone tries to add
>> > > > support for them.
>> > > >
>> > > > Signed-off-by: Alexandru Gheorghe
>> > > > ---
>> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
>> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
>> > > > drivers/gpu/drm/drm_fourcc.c | 52
>> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
>> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
>> > > > include/drm/drm_fourcc.h | 2 +
>> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/drm_atomic.c
>> > > > b/drivers/gpu/drm/drm_atomic.c
>> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
>> > > > --- a/drivers/gpu/drm/drm_atomic.c
>> > > > +++ b/drivers/gpu/drm/drm_atomic.c
>> > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
>> > > > drm_plane *plane,
>> > > > return -ENOSPC;
>> > > > }
>> > > >
>> > > > + /* Make sure source coordinates are a multiple of tile sizes */
>> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
>> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
>> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do
>> > > > not meet tile restrictions",
>> > > > +plane->base.id, plane->name);
>> > > > + return -EINVAL;
>> > > > + }
>> > >
>> > > Feels rather wrong to me to put such hardware specific limitations into
>> > > the core.
>> >
>> > Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
>> > work.
>>
>> If that guy is supposed to give you a tile aligned address then it could
>> just do that and leave it up to the driver to deal with the remainder of
>> the coordinates?
>>
>> >
>> > An alternative, would be to include it in the driver and put an WARN
>> > in drm_fb_cma_get_gem_addr in case someone else uses it with a
>> > src_x/src_y tile multiples.
>> >
>> > What do you think about that ?
>> >
>> > >
>> > > > +
>> > > > if (plane_switching_crtc(state->state, plane, state)) {
>> > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
>> > > > directly\n",
>> > > > plane->base.id, plane->name);
>> > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
>> > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
>> > > > index 47e0e2f6642d..4d8052adce67 100644
>> > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
>> > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>> > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
>> > > > drm_framebuffer *fb,
>> > > > struct drm_gem_cma_object *obj;
>> > > > dma_addr_t paddr;
>> > > > u8 h_div = 1, v_div = 1;
>> > > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
>> > > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
>> > > >
>> > > > obj = drm_fb_cma_get_gem_obj(fb, plane);
>> > > > if (!obj)
>> > > > @@ -99,8 +101,13 @@ dma_addr_t
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Wed, Aug 22, 2018 at 04:45:34PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe wrote:
> > On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> > > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > > The previous patch added tile_w and tile_h, which represent the
> > > > horizontal and vertical sizes of a tile.
> > > >
> > > > This one uses that to plumb through drm core in order to be able to
> > > > handle linear tile formats without the need for drivers to roll up
> > > > their own implementation.
> > > >
> > > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> > > > bytes per pixel and where tiles are laid out in a linear manner.
> > > >
> > > > Now what are the restrictions:
> > > >
> > > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > > pixels. Due to this the places where the pitch is checked/used need to
> > > > be updated to take into consideration the tile_w, tile_h and
> > > > tile_size.
> > > > tile_size = cpp * tile_w * tile_h
> > > >
> > > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > > > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> > > > when computing the start address.
> > > >
> > > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > > didn't miss anything nothing should change.
> > > >
> > > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > > should be handle I kind of assumed that tile_h/tile_w will have to be
> > > > divided by horizontal/subsampling. Anyway, I think it's best to just
> > > > put an warning in there and handle it when someone tries to add
> > > > support for them.
> > > >
> > > > Signed-off-by: Alexandru Gheorghe
> > > > ---
> > > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > > drivers/gpu/drm/drm_fourcc.c | 52
> > > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > > include/drm/drm_fourcc.h | 2 +
> > > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > > --- a/drivers/gpu/drm/drm_atomic.c
> > > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct
> > > > drm_plane *plane,
> > > > return -ENOSPC;
> > > > }
> > > >
> > > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do
> > > > not meet tile restrictions",
> > > > +plane->base.id, plane->name);
> > > > + return -EINVAL;
> > > > + }
> > >
> > > Feels rather wrong to me to put such hardware specific limitations into
> > > the core.
> >
> > Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
> > work.
>
> If that guy is supposed to give you a tile aligned address then it could
> just do that and leave it up to the driver to deal with the remainder of
> the coordinates?
>
> >
> > An alternative, would be to include it in the driver and put an WARN
> > in drm_fb_cma_get_gem_addr in case someone else uses it with a
> > src_x/src_y tile multiples.
> >
> > What do you think about that ?
> >
> > >
> > > > +
> > > > if (plane_switching_crtc(state->state, plane, state)) {
> > > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC
> > > > directly\n",
> > > > plane->base.id, plane->name);
> > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > index 47e0e2f6642d..4d8052adce67 100644
> > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > drm_framebuffer *fb,
> > > > struct drm_gem_cma_object *obj;
> > > > dma_addr_t paddr;
> > > > u8 h_div = 1, v_div = 1;
> > > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > > >
> > > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > > if (!obj)
> > > > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > > drm_framebuffer *fb,
> > > > v_div = fb->format->vsub;
> > > > }
> > > >
> > > > - paddr +=
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Wed, Aug 22, 2018 at 02:36:06PM +0100, Alexandru-Cosmin Gheorghe wrote:
> On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> > On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > > The previous patch added tile_w and tile_h, which represent the
> > > horizontal and vertical sizes of a tile.
> > >
> > > This one uses that to plumb through drm core in order to be able to
> > > handle linear tile formats without the need for drivers to roll up
> > > their own implementation.
> > >
> > > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> > > bytes per pixel and where tiles are laid out in a linear manner.
> > >
> > > Now what are the restrictions:
> > >
> > > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > > pixels. Due to this the places where the pitch is checked/used need to
> > > be updated to take into consideration the tile_w, tile_h and
> > > tile_size.
> > > tile_size = cpp * tile_w * tile_h
> > >
> > > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> > > when computing the start address.
> > >
> > > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > > didn't miss anything nothing should change.
> > >
> > > Regarding multi-planar linear tile formats, I'm not sure how those
> > > should be handle I kind of assumed that tile_h/tile_w will have to be
> > > divided by horizontal/subsampling. Anyway, I think it's best to just
> > > put an warning in there and handle it when someone tries to add
> > > support for them.
> > >
> > > Signed-off-by: Alexandru Gheorghe
> > > ---
> > > drivers/gpu/drm/drm_atomic.c | 8 +++
> > > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > > drivers/gpu/drm/drm_fourcc.c | 52
> > > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > > include/drm/drm_fourcc.h | 2 +
> > > 6 files changed, 94 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 3eb061e11e2e..7a3e893a4cd1 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> > > *plane,
> > > return -ENOSPC;
> > > }
> > >
> > > + /* Make sure source coordinates are a multiple of tile sizes */
> > > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> > > tile restrictions",
> > > + plane->base.id, plane->name);
> > > + return -EINVAL;
> > > + }
> >
> > Feels rather wrong to me to put such hardware specific limitations into
> > the core.
>
> Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
> work.
If that guy is supposed to give you a tile aligned address then it could
just do that and leave it up to the driver to deal with the remainder of
the coordinates?
>
> An alternative, would be to include it in the driver and put an WARN
> in drm_fb_cma_get_gem_addr in case someone else uses it with a
> src_x/src_y tile multiples.
>
> What do you think about that ?
>
> >
> > > +
> > > if (plane_switching_crtc(state->state, plane, state)) {
> > > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> > >plane->base.id, plane->name);
> > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > index 47e0e2f6642d..4d8052adce67 100644
> > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > drm_framebuffer *fb,
> > > struct drm_gem_cma_object *obj;
> > > dma_addr_t paddr;
> > > u8 h_div = 1, v_div = 1;
> > > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> > >
> > > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > if (!obj)
> > > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > > drm_framebuffer *fb,
> > > v_div = fb->format->vsub;
> > > }
> > >
> > > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> > > - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> > > + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> > > + / h_div;
> > > + /*
> > > + * For tile formats pitches are expected to cover at least
> > > + * width * tile_h pixels
> > > + */
> > > + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >>
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Wed, Aug 22, 2018 at 04:18:54PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> > The previous patch added tile_w and tile_h, which represent the
> > horizontal and vertical sizes of a tile.
> >
> > This one uses that to plumb through drm core in order to be able to
> > handle linear tile formats without the need for drivers to roll up
> > their own implementation.
> >
> > This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> > is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> > bytes per pixel and where tiles are laid out in a linear manner.
> >
> > Now what are the restrictions:
> >
> > 1. Pitch in bytes is expected to cover at least tile_h * width in
> > pixels. Due to this the places where the pitch is checked/used need to
> > be updated to take into consideration the tile_w, tile_h and
> > tile_size.
> > tile_size = cpp * tile_w * tile_h
> >
> > 2. When doing source cropping plane_src_x/y need to be a multiple of
> > tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> > when computing the start address.
> >
> > For all non-tiled formats the tile_w and tile_h will be 1, so if I
> > didn't miss anything nothing should change.
> >
> > Regarding multi-planar linear tile formats, I'm not sure how those
> > should be handle I kind of assumed that tile_h/tile_w will have to be
> > divided by horizontal/subsampling. Anyway, I think it's best to just
> > put an warning in there and handle it when someone tries to add
> > support for them.
> >
> > Signed-off-by: Alexandru Gheorghe
> > ---
> > drivers/gpu/drm/drm_atomic.c | 8 +++
> > drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> > drivers/gpu/drm/drm_fourcc.c | 52
> > drivers/gpu/drm/drm_framebuffer.c| 19 +--
> > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> > include/drm/drm_fourcc.h | 2 +
> > 6 files changed, 94 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 3eb061e11e2e..7a3e893a4cd1 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> > *plane,
> > return -ENOSPC;
> > }
> >
> > + /* Make sure source coordinates are a multiple of tile sizes */
> > + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> > + (state->src_y >> 16) % state->fb->format->tile_h) {
> > + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> > tile restrictions",
> > +plane->base.id, plane->name);
> > + return -EINVAL;
> > + }
>
> Feels rather wrong to me to put such hardware specific limitations into
> the core.
Yeah, the problem is that otherwise drm_fb_cma_get_gem_addr wouldn't
work.
An alternative, would be to include it in the driver and put an WARN
in drm_fb_cma_get_gem_addr in case someone else uses it with a
src_x/src_y tile multiples.
What do you think about that ?
>
> > +
> > if (plane_switching_crtc(state->state, plane, state)) {
> > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
> > plane->base.id, plane->name);
> > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> > b/drivers/gpu/drm/drm_fb_cma_helper.c
> > index 47e0e2f6642d..4d8052adce67 100644
> > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> > *fb,
> > struct drm_gem_cma_object *obj;
> > dma_addr_t paddr;
> > u8 h_div = 1, v_div = 1;
> > + u32 tile_w = drm_format_tile_width(fb->format, plane);
> > + u32 tile_h = drm_format_tile_height(fb->format, plane);
> >
> > obj = drm_fb_cma_get_gem_obj(fb, plane);
> > if (!obj)
> > @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct
> > drm_framebuffer *fb,
> > v_div = fb->format->vsub;
> > }
> >
> > - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> > - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> > + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> > + / h_div;
> > + /*
> > +* For tile formats pitches are expected to cover at least
> > +* width * tile_h pixels
> > +*/
> > + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
> >
> > return paddr;
> > }
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index f55cd93ba2d0..d6c9c5aa4036 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t
> > format, int plane)
> > return height / info->vsub;
> > }
> >
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> The previous patch added tile_w and tile_h, which represent the
> horizontal and vertical sizes of a tile.
>
> This one uses that to plumb through drm core in order to be able to
> handle linear tile formats without the need for drivers to roll up
> their own implementation.
>
> This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> bytes per pixel and where tiles are laid out in a linear manner.
>
> Now what are the restrictions:
>
> 1. Pitch in bytes is expected to cover at least tile_h * width in
> pixels. Due to this the places where the pitch is checked/used need to
> be updated to take into consideration the tile_w, tile_h and
> tile_size.
> tile_size = cpp * tile_w * tile_h
>
> 2. When doing source cropping plane_src_x/y need to be a multiple of
> tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> when computing the start address.
>
> For all non-tiled formats the tile_w and tile_h will be 1, so if I
> didn't miss anything nothing should change.
>
> Regarding multi-planar linear tile formats, I'm not sure how those
> should be handle I kind of assumed that tile_h/tile_w will have to be
> divided by horizontal/subsampling. Anyway, I think it's best to just
> put an warning in there and handle it when someone tries to add
> support for them.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drivers/gpu/drm/drm_atomic.c | 8 +++
> drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> drivers/gpu/drm/drm_fourcc.c | 52
> drivers/gpu/drm/drm_framebuffer.c| 19 +--
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> include/drm/drm_fourcc.h | 2 +
> 6 files changed, 94 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3eb061e11e2e..7a3e893a4cd1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> *plane,
> return -ENOSPC;
> }
>
> + /* Make sure source coordinates are a multiple of tile sizes */
> + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> + (state->src_y >> 16) % state->fb->format->tile_h) {
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> tile restrictions",
> + plane->base.id, plane->name);
> + return -EINVAL;
> + }
Feels rather wrong to me to put such hardware specific limitations into
the core.
> +
> if (plane_switching_crtc(state->state, plane, state)) {
> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
>plane->base.id, plane->name);
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 47e0e2f6642d..4d8052adce67 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> struct drm_gem_cma_object *obj;
> dma_addr_t paddr;
> u8 h_div = 1, v_div = 1;
> + u32 tile_w = drm_format_tile_width(fb->format, plane);
> + u32 tile_h = drm_format_tile_height(fb->format, plane);
>
> obj = drm_fb_cma_get_gem_obj(fb, plane);
> if (!obj)
> @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> v_div = fb->format->vsub;
> }
>
> - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> + / h_div;
> + /*
> + * For tile formats pitches are expected to cover at least
> + * width * tile_h pixels
> + */
> + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
>
> return paddr;
> }
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index f55cd93ba2d0..d6c9c5aa4036 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t format,
> int plane)
> return height / info->vsub;
> }
> EXPORT_SYMBOL(drm_format_plane_height);
> +
> +/**
> + * drm_format_tile_width - width of a tile for tile formats, should be 1 for
> all
> + * non-tiled formats.
> + * @format: pixel format
> + * @plane: plane index
> + *
> + * Returns:
> + * The width of a tile, depending on the plane index and horizontal
> sub-sampling
> + */
> +uint32_t drm_format_tile_width(const struct drm_format_info *info, int plane)
> +{
> + WARN_ON(!info->tile_w);
> + if (plane == 0 || info->tile_w
Re: [PATCH v2 4/5] drm: Add support for handling linear tile formats
On Tue, Aug 21, 2018 at 07:30:03PM +0100, Alexandru Gheorghe wrote:
> The previous patch added tile_w and tile_h, which represent the
> horizontal and vertical sizes of a tile.
>
> This one uses that to plumb through drm core in order to be able to
> handle linear tile formats without the need for drivers to roll up
> their own implementation.
>
> This patch had been written with Mali-dp X0L2 and X0L0 in mind which
> is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
> bytes per pixel and where tiles are laid out in a linear manner.
>
> Now what are the restrictions:
>
> 1. Pitch in bytes is expected to cover at least tile_h * width in
> pixels.
This reads wrong for the first time reader. Maybe make the 'cpp'
explicit like you do later on?
> Due to this the places where the pitch is checked/used need to
> be updated to take into consideration the tile_w, tile_h and
> tile_size.
> tile_size = cpp * tile_w * tile_h
>
> 2. When doing source cropping plane_src_x/y need to be a multiple of
> tile_w/tile_h and we need to take into consideration the tile_w/tile_h
> when computing the start address.
>
> For all non-tiled formats the tile_w and tile_h will be 1, so if I
> didn't miss anything nothing should change.
>
> Regarding multi-planar linear tile formats, I'm not sure how those
> should be handle I kind of assumed that tile_h/tile_w will have to be
> divided by horizontal/subsampling. Anyway, I think it's best to just
> put an warning in there and handle it when someone tries to add
> support for them.
>
> Signed-off-by: Alexandru Gheorghe
> ---
> drivers/gpu/drm/drm_atomic.c | 8 +++
> drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
> drivers/gpu/drm/drm_fourcc.c | 52
> drivers/gpu/drm/drm_framebuffer.c| 19 +--
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
> include/drm/drm_fourcc.h | 2 +
> 6 files changed, 94 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3eb061e11e2e..7a3e893a4cd1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
> *plane,
> return -ENOSPC;
> }
>
> + /* Make sure source coordinates are a multiple of tile sizes */
> + if ((state->src_x >> 16) % state->fb->format->tile_w ||
> + (state->src_y >> 16) % state->fb->format->tile_h) {
> + DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
> tile restrictions",
> + plane->base.id, plane->name);
> + return -EINVAL;
> + }
> +
> if (plane_switching_crtc(state->state, plane, state)) {
> DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
>plane->base.id, plane->name);
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 47e0e2f6642d..4d8052adce67 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> struct drm_gem_cma_object *obj;
> dma_addr_t paddr;
> u8 h_div = 1, v_div = 1;
> + u32 tile_w = drm_format_tile_width(fb->format, plane);
> + u32 tile_h = drm_format_tile_height(fb->format, plane);
>
> obj = drm_fb_cma_get_gem_obj(fb, plane);
> if (!obj)
> @@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
> *fb,
> v_div = fb->format->vsub;
> }
>
> - paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
> - paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
> + paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
> + / h_div;
I think you can keep the division by h_div on the same line, you're not
spilling over the limit that much compared to the next line of code.
> + /*
> + * For tile formats pitches are expected to cover at least
> + * width * tile_h pixels
> + */
> + paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
>
> return paddr;
> }
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index f55cd93ba2d0..d6c9c5aa4036 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t format,
> int plane)
> return height / info->vsub;
> }
> EXPORT_SYMBOL(drm_format_plane_height);
> +
> +/**
> + * drm_format_tile_width - width of a tile for tile formats, should be 1 for
> all
> + * non-tiled formats.
> + * @format: pixel format
> + * @plane: plane index
> + *
> + * Returns:
> + * The width of a tile, depending on the plane index and horizontal
>
[PATCH v2 4/5] drm: Add support for handling linear tile formats
The previous patch added tile_w and tile_h, which represent the
horizontal and vertical sizes of a tile.
This one uses that to plumb through drm core in order to be able to
handle linear tile formats without the need for drivers to roll up
their own implementation.
This patch had been written with Mali-dp X0L2 and X0L0 in mind which
is a 1 plane YCbCr 420 format with a 2x2 tile, that uses in average 2
bytes per pixel and where tiles are laid out in a linear manner.
Now what are the restrictions:
1. Pitch in bytes is expected to cover at least tile_h * width in
pixels. Due to this the places where the pitch is checked/used need to
be updated to take into consideration the tile_w, tile_h and
tile_size.
tile_size = cpp * tile_w * tile_h
2. When doing source cropping plane_src_x/y need to be a multiple of
tile_w/tile_h and we need to take into consideration the tile_w/tile_h
when computing the start address.
For all non-tiled formats the tile_w and tile_h will be 1, so if I
didn't miss anything nothing should change.
Regarding multi-planar linear tile formats, I'm not sure how those
should be handle I kind of assumed that tile_h/tile_w will have to be
divided by horizontal/subsampling. Anyway, I think it's best to just
put an warning in there and handle it when someone tries to add
support for them.
Signed-off-by: Alexandru Gheorghe
---
drivers/gpu/drm/drm_atomic.c | 8 +++
drivers/gpu/drm/drm_fb_cma_helper.c | 11 -
drivers/gpu/drm/drm_fourcc.c | 52
drivers/gpu/drm/drm_framebuffer.c| 19 +--
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 10 ++--
include/drm/drm_fourcc.h | 2 +
6 files changed, 94 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3eb061e11e2e..7a3e893a4cd1 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1087,6 +1087,14 @@ static int drm_atomic_plane_check(struct drm_plane
*plane,
return -ENOSPC;
}
+ /* Make sure source coordinates are a multiple of tile sizes */
+ if ((state->src_x >> 16) % state->fb->format->tile_w ||
+ (state->src_y >> 16) % state->fb->format->tile_h) {
+ DRM_DEBUG_ATOMIC("[PLANE:%d:%s] Source coordinates do not meet
tile restrictions",
+plane->base.id, plane->name);
+ return -EINVAL;
+ }
+
if (plane_switching_crtc(state->state, plane, state)) {
DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",
plane->base.id, plane->name);
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
b/drivers/gpu/drm/drm_fb_cma_helper.c
index 47e0e2f6642d..4d8052adce67 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -87,6 +87,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
struct drm_gem_cma_object *obj;
dma_addr_t paddr;
u8 h_div = 1, v_div = 1;
+ u32 tile_w = drm_format_tile_width(fb->format, plane);
+ u32 tile_h = drm_format_tile_height(fb->format, plane);
obj = drm_fb_cma_get_gem_obj(fb, plane);
if (!obj)
@@ -99,8 +101,13 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer
*fb,
v_div = fb->format->vsub;
}
- paddr += (fb->format->cpp[plane] * (state->src_x >> 16)) / h_div;
- paddr += (fb->pitches[plane] * (state->src_y >> 16)) / v_div;
+ paddr += (fb->format->cpp[plane] * tile_w * (state->src_x >> 16))
+ / h_div;
+ /*
+* For tile formats pitches are expected to cover at least
+* width * tile_h pixels
+*/
+ paddr += ((fb->pitches[plane] / tile_h) * (state->src_y >> 16)) / v_div;
return paddr;
}
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index f55cd93ba2d0..d6c9c5aa4036 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -557,3 +557,55 @@ int drm_format_plane_height(int height, uint32_t format,
int plane)
return height / info->vsub;
}
EXPORT_SYMBOL(drm_format_plane_height);
+
+/**
+ * drm_format_tile_width - width of a tile for tile formats, should be 1 for
all
+ * non-tiled formats.
+ * @format: pixel format
+ * @plane: plane index
+ *
+ * Returns:
+ * The width of a tile, depending on the plane index and horizontal
sub-sampling
+ */
+uint32_t drm_format_tile_width(const struct drm_format_info *info, int plane)
+{
+ WARN_ON(!info->tile_w);
+ if (plane == 0 || info->tile_w == 1)
+ return info->tile_w;
+
+ /*
+* Multi planar tiled formats have never been tested, check that
+* buffer restrictions and source cropping meet the format layout
+* expectations.
+*/
+ WARN_ON("Multi-planar tiled formats unsupported");
+
