Re: [PATCH v2] drm: Check actual format for legacy pageflip.
On Mon, Jan 11, 2021 at 04:28:31PM -0500, Alex Deucher wrote: > On Mon, Jan 11, 2021 at 11:39 AM Bas Nieuwenhuizen > wrote: > > > > On Mon, Jan 11, 2021 at 4:02 PM Alex Deucher wrote: > > > > > > On Sat, Jan 9, 2021 at 9:11 PM Bas Nieuwenhuizen > > > wrote: > > > > > > > > With modifiers one can actually have different format_info structs > > > > for the same format, which now matters for AMDGPU since we convert > > > > implicit modifiers to explicit modifiers with multiple planes. > > > > > > > > I checked other drivers and it doesn't look like they end up triggering > > > > this case so I think this is safe to relax. > > > > > > > > Signed-off-by: Bas Nieuwenhuizen > > > > Reviewed-by: Daniel Vetter > > > > Reviewed-by: Zhan Liu > > > > Acked-by: Christian König > > > > Acked-by: Alex Deucher > > > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for > > > > converted metadata.") > > > > > > Do you have commit rights to drm-misc or do you need someone to commit > > > this for you? > > > > I don't have commit rights so if the patch could be committed for me > > that would be appreciated! > > Pushed to drm-misc-fixes. Thanks! > > If you want access to drm-misc, I don't see any reason you shouldn't have it. There's some old-school bash tooling involved since we're (not yet, I can hope) doing gitlab MR: https://drm.pages.freedesktop.org/maintainer-tools/getting-started.html Otherwise makes sense imo. -Daniel > > Alex > > > > > > > > Thanks! > > > > > > Alex > > > > > > > --- > > > > drivers/gpu/drm/drm_plane.c | 9 - > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > > > index e6231947f987..a0cb746bcb0a 100644 > > > > --- a/drivers/gpu/drm/drm_plane.c > > > > +++ b/drivers/gpu/drm/drm_plane.c > > > > @@ -1163,7 +1163,14 @@ int drm_mode_page_flip_ioctl(struct drm_device > > > > *dev, > > > > if (ret) > > > > goto out; > > > > > > > > - if (old_fb->format != fb->format) { > > > > + /* > > > > +* Only check the FOURCC format code, excluding modifiers. This > > > > is > > > > +* enough for all legacy drivers. Atomic drivers have their own > > > > +* checks in their ->atomic_check implementation, which will > > > > +* return -EINVAL if any hw or driver constraint is violated due > > > > +* to modifier changes. > > > > +*/ > > > > + if (old_fb->format->format != fb->format->format) { > > > > DRM_DEBUG_KMS("Page flip is not allowed to change frame > > > > buffer format.\n"); > > > > ret = -EINVAL; > > > > goto out; > > > > -- > > > > 2.29.2 > > > > > > > > ___ > > > > amd-gfx mailing list > > > > amd-...@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: Check actual format for legacy pageflip.
On Mon, Jan 11, 2021 at 11:39 AM Bas Nieuwenhuizen wrote: > > On Mon, Jan 11, 2021 at 4:02 PM Alex Deucher wrote: > > > > On Sat, Jan 9, 2021 at 9:11 PM Bas Nieuwenhuizen > > wrote: > > > > > > With modifiers one can actually have different format_info structs > > > for the same format, which now matters for AMDGPU since we convert > > > implicit modifiers to explicit modifiers with multiple planes. > > > > > > I checked other drivers and it doesn't look like they end up triggering > > > this case so I think this is safe to relax. > > > > > > Signed-off-by: Bas Nieuwenhuizen > > > Reviewed-by: Daniel Vetter > > > Reviewed-by: Zhan Liu > > > Acked-by: Christian König > > > Acked-by: Alex Deucher > > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for converted > > > metadata.") > > > > Do you have commit rights to drm-misc or do you need someone to commit > > this for you? > > I don't have commit rights so if the patch could be committed for me > that would be appreciated! Pushed to drm-misc-fixes. Thanks! If you want access to drm-misc, I don't see any reason you shouldn't have it. Alex > > > > Thanks! > > > > Alex > > > > > --- > > > drivers/gpu/drm/drm_plane.c | 9 - > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > > index e6231947f987..a0cb746bcb0a 100644 > > > --- a/drivers/gpu/drm/drm_plane.c > > > +++ b/drivers/gpu/drm/drm_plane.c > > > @@ -1163,7 +1163,14 @@ int drm_mode_page_flip_ioctl(struct drm_device > > > *dev, > > > if (ret) > > > goto out; > > > > > > - if (old_fb->format != fb->format) { > > > + /* > > > +* Only check the FOURCC format code, excluding modifiers. This is > > > +* enough for all legacy drivers. Atomic drivers have their own > > > +* checks in their ->atomic_check implementation, which will > > > +* return -EINVAL if any hw or driver constraint is violated due > > > +* to modifier changes. > > > +*/ > > > + if (old_fb->format->format != fb->format->format) { > > > DRM_DEBUG_KMS("Page flip is not allowed to change frame > > > buffer format.\n"); > > > ret = -EINVAL; > > > goto out; > > > -- > > > 2.29.2 > > > > > > ___ > > > amd-gfx mailing list > > > amd-...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: Check actual format for legacy pageflip.
On Mon, Jan 11, 2021 at 4:02 PM Alex Deucher wrote: > > On Sat, Jan 9, 2021 at 9:11 PM Bas Nieuwenhuizen > wrote: > > > > With modifiers one can actually have different format_info structs > > for the same format, which now matters for AMDGPU since we convert > > implicit modifiers to explicit modifiers with multiple planes. > > > > I checked other drivers and it doesn't look like they end up triggering > > this case so I think this is safe to relax. > > > > Signed-off-by: Bas Nieuwenhuizen > > Reviewed-by: Daniel Vetter > > Reviewed-by: Zhan Liu > > Acked-by: Christian König > > Acked-by: Alex Deucher > > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for converted > > metadata.") > > Do you have commit rights to drm-misc or do you need someone to commit > this for you? I don't have commit rights so if the patch could be committed for me that would be appreciated! > > Thanks! > > Alex > > > --- > > drivers/gpu/drm/drm_plane.c | 9 - > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > > index e6231947f987..a0cb746bcb0a 100644 > > --- a/drivers/gpu/drm/drm_plane.c > > +++ b/drivers/gpu/drm/drm_plane.c > > @@ -1163,7 +1163,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, > > if (ret) > > goto out; > > > > - if (old_fb->format != fb->format) { > > + /* > > +* Only check the FOURCC format code, excluding modifiers. This is > > +* enough for all legacy drivers. Atomic drivers have their own > > +* checks in their ->atomic_check implementation, which will > > +* return -EINVAL if any hw or driver constraint is violated due > > +* to modifier changes. > > +*/ > > + if (old_fb->format->format != fb->format->format) { > > DRM_DEBUG_KMS("Page flip is not allowed to change frame > > buffer format.\n"); > > ret = -EINVAL; > > goto out; > > -- > > 2.29.2 > > > > ___ > > amd-gfx mailing list > > amd-...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: Check actual format for legacy pageflip.
On Sat, Jan 9, 2021 at 9:11 PM Bas Nieuwenhuizen wrote: > > With modifiers one can actually have different format_info structs > for the same format, which now matters for AMDGPU since we convert > implicit modifiers to explicit modifiers with multiple planes. > > I checked other drivers and it doesn't look like they end up triggering > this case so I think this is safe to relax. > > Signed-off-by: Bas Nieuwenhuizen > Reviewed-by: Daniel Vetter > Reviewed-by: Zhan Liu > Acked-by: Christian König > Acked-by: Alex Deucher > Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for converted > metadata.") Do you have commit rights to drm-misc or do you need someone to commit this for you? Thanks! Alex > --- > drivers/gpu/drm/drm_plane.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index e6231947f987..a0cb746bcb0a 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -1163,7 +1163,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, > if (ret) > goto out; > > - if (old_fb->format != fb->format) { > + /* > +* Only check the FOURCC format code, excluding modifiers. This is > +* enough for all legacy drivers. Atomic drivers have their own > +* checks in their ->atomic_check implementation, which will > +* return -EINVAL if any hw or driver constraint is violated due > +* to modifier changes. > +*/ > + if (old_fb->format->format != fb->format->format) { > DRM_DEBUG_KMS("Page flip is not allowed to change frame > buffer format.\n"); > ret = -EINVAL; > goto out; > -- > 2.29.2 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: Check actual format for legacy pageflip.
With modifiers one can actually have different format_info structs for the same format, which now matters for AMDGPU since we convert implicit modifiers to explicit modifiers with multiple planes. I checked other drivers and it doesn't look like they end up triggering this case so I think this is safe to relax. Signed-off-by: Bas Nieuwenhuizen Reviewed-by: Daniel Vetter Reviewed-by: Zhan Liu Acked-by: Christian König Acked-by: Alex Deucher Fixes: 816853f9dc40 ("drm/amd/display: Set new format info for converted metadata.") --- drivers/gpu/drm/drm_plane.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index e6231947f987..a0cb746bcb0a 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1163,7 +1163,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, if (ret) goto out; - if (old_fb->format != fb->format) { + /* +* Only check the FOURCC format code, excluding modifiers. This is +* enough for all legacy drivers. Atomic drivers have their own +* checks in their ->atomic_check implementation, which will +* return -EINVAL if any hw or driver constraint is violated due +* to modifier changes. +*/ + if (old_fb->format->format != fb->format->format) { DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n"); ret = -EINVAL; goto out; -- 2.29.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel