| Message ID | 20240913071036.574782-2-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/nouveau: Add drm_panic support for nv50+ | expand |
Jocelyn Falempe <jfalempe@redhat.com> writes: Hello Jocelyn, > Add support for ABGR2101010, used by the nouveau driver. > > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > --- > drivers/gpu/drm/drm_panic.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c > index 74412b7bf936..0a9ecc1380d2 100644 > --- a/drivers/gpu/drm/drm_panic.c > +++ b/drivers/gpu/drm/drm_panic.c > @@ -209,6 +209,14 @@ static u32 convert_xrgb8888_to_argb2101010(u32 pix) > return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); > } > > +static u32 convert_xrgb8888_to_abgr2101010(u32 pix) > +{ > + pix = ((pix & 0x00FF0000) >> 14) | > + ((pix & 0x0000FF00) << 4) | > + ((pix & 0x000000FF) << 22); > + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); > +} Maybe we can move this format conversion helper and the others in the driver to drivers/gpu/drm/drm_format_helper.c ? > + > /* > * convert_from_xrgb8888 - convert one pixel from xrgb8888 to the desired format > * @color: input color, in xrgb8888 format > @@ -242,6 +250,8 @@ static u32 convert_from_xrgb8888(u32 color, u32 format) > return convert_xrgb8888_to_xrgb2101010(color); > case DRM_FORMAT_ARGB2101010: > return convert_xrgb8888_to_argb2101010(color); > + case DRM_FORMAT_ABGR2101010: > + return convert_xrgb8888_to_abgr2101010(color); > default: > WARN_ONCE(1, "Can't convert to %p4cc\n", &format); > return 0; > -- > 2.46.0 > The patch looks good to me. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
On 13/09/2024 09:22, Javier Martinez Canillas wrote: > Jocelyn Falempe <jfalempe@redhat.com> writes: > > Hello Jocelyn, > >> Add support for ABGR2101010, used by the nouveau driver. >> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> --- >> drivers/gpu/drm/drm_panic.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c >> index 74412b7bf936..0a9ecc1380d2 100644 >> --- a/drivers/gpu/drm/drm_panic.c >> +++ b/drivers/gpu/drm/drm_panic.c >> @@ -209,6 +209,14 @@ static u32 convert_xrgb8888_to_argb2101010(u32 pix) >> return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); >> } >> >> +static u32 convert_xrgb8888_to_abgr2101010(u32 pix) >> +{ >> + pix = ((pix & 0x00FF0000) >> 14) | >> + ((pix & 0x0000FF00) << 4) | >> + ((pix & 0x000000FF) << 22); >> + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); >> +} > > Maybe we can move this format conversion helper and the others in the > driver to drivers/gpu/drm/drm_format_helper.c ? I think there are still a few issues with that. First is that drm_format_helper.c is in a separate module, so you can't call its functions from the main drm module, where drm_panic is. In my drm_log series, https://patchwork.freedesktop.org/series/136789/ I moved this to drm_draw.c, and maybe drm_format_helper could re-use that ? > >> + >> /* >> * convert_from_xrgb8888 - convert one pixel from xrgb8888 to the desired format >> * @color: input color, in xrgb8888 format >> @@ -242,6 +250,8 @@ static u32 convert_from_xrgb8888(u32 color, u32 format) >> return convert_xrgb8888_to_xrgb2101010(color); >> case DRM_FORMAT_ARGB2101010: >> return convert_xrgb8888_to_argb2101010(color); >> + case DRM_FORMAT_ABGR2101010: >> + return convert_xrgb8888_to_abgr2101010(color); >> default: >> WARN_ONCE(1, "Can't convert to %p4cc\n", &format); >> return 0; >> -- >> 2.46.0 >> > > > The patch looks good to me. > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >
Jocelyn Falempe <jfalempe@redhat.com> writes: > On 13/09/2024 09:22, Javier Martinez Canillas wrote: >> Jocelyn Falempe <jfalempe@redhat.com> writes: >> >> Hello Jocelyn, >> >>> Add support for ABGR2101010, used by the nouveau driver. >>> >>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >>> --- >>> drivers/gpu/drm/drm_panic.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c >>> index 74412b7bf936..0a9ecc1380d2 100644 >>> --- a/drivers/gpu/drm/drm_panic.c >>> +++ b/drivers/gpu/drm/drm_panic.c >>> @@ -209,6 +209,14 @@ static u32 convert_xrgb8888_to_argb2101010(u32 pix) >>> return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); >>> } >>> >>> +static u32 convert_xrgb8888_to_abgr2101010(u32 pix) >>> +{ >>> + pix = ((pix & 0x00FF0000) >> 14) | >>> + ((pix & 0x0000FF00) << 4) | >>> + ((pix & 0x000000FF) << 22); >>> + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); >>> +} >> >> Maybe we can move this format conversion helper and the others in the >> driver to drivers/gpu/drm/drm_format_helper.c ? > > I think there are still a few issues with that. First is that > drm_format_helper.c is in a separate module, so you can't call its > functions from the main drm module, where drm_panic is. > I see. > In my drm_log series, https://patchwork.freedesktop.org/series/136789/ I > moved this to drm_draw.c, and maybe drm_format_helper could re-use that ? > That makes sense to me as well. Thomas, what do you think ?
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 74412b7bf936..0a9ecc1380d2 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -209,6 +209,14 @@ static u32 convert_xrgb8888_to_argb2101010(u32 pix) return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); } +static u32 convert_xrgb8888_to_abgr2101010(u32 pix) +{ + pix = ((pix & 0x00FF0000) >> 14) | + ((pix & 0x0000FF00) << 4) | + ((pix & 0x000000FF) << 22); + return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03); +} + /* * convert_from_xrgb8888 - convert one pixel from xrgb8888 to the desired format * @color: input color, in xrgb8888 format @@ -242,6 +250,8 @@ static u32 convert_from_xrgb8888(u32 color, u32 format) return convert_xrgb8888_to_xrgb2101010(color); case DRM_FORMAT_ARGB2101010: return convert_xrgb8888_to_argb2101010(color); + case DRM_FORMAT_ABGR2101010: + return convert_xrgb8888_to_abgr2101010(color); default: WARN_ONCE(1, "Can't convert to %p4cc\n", &format); return 0;
Add support for ABGR2101010, used by the nouveau driver. Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- drivers/gpu/drm/drm_panic.c | 10 ++++++++++ 1 file changed, 10 insertions(+)