All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: "Bartlomiej Zolnierkiewicz" <b.zolnierkie@samsung.com>,
	"Eric Anholt" <eric@anholt.net>,
	"Noralf Trønnes" <noralf@tronnes.org>,
	"Dave Airlie" <airlied@redhat.com>,
	"David Lechner" <david@lechnology.com>
Cc: linux-fbdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org
Subject: [PATCH v3] drm/fb-helper: Scale back depth to supported maximum
Date: Thu, 10 Jan 2019 09:24:03 +0000	[thread overview]
Message-ID: <20190110092403.28324-1-linus.walleij@linaro.org> (raw)

The following happened when migrating an old fbdev driver to DRM:

The Integrator/CP PL111 supports 16BPP but only ARGB1555/ABGR1555
or XRGB1555/XBGR1555 i.e. the maximum depth is 15.

This makes the initialization of the framebuffer fail since
the code in drm_fb_helper_single_fb_probe() assigns the same value
to sizes.surface_bpp and sizes.surface_depth. I.e. it simply assumes
a 1-to-1 mapping between BPP and depth, which is true in most cases
but not for this hardware that only support odd formats.

To support the odd case of a driver supporting 16BPP with only 15
bits of depth, this patch will make the code loop over the formats
supported on the primary plane on each CRTC managed by the FB
helper and cap the depth to the maximum supported on any primary
plane.

On the PL110 Integrator, this makes drm_mode_legacy_fb_format()
select DRM_FORMAT_XRGB1555 which is acceptable for this driver, and
thus we get framebuffer, penguin and console on the Integrator/CP.

Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Rebased and tested with v5.0-rc1
- Skip any formats with fmt->depth = 0
- Collected Daniels review tag.
ChangeLog v1->v2:
- Loop over the CRTCs managed by the helper and check the
  crtc->primary on each CRTC for the applicable formats and
  thus depths.
- Skip over YUV formats. The framebuffer emulation cannot
  handle these formats.

The v1 was sent some while back in february and I only recently
got back to fixing this up to support the last CLCD displays.
It was agreed that it is probably best to augment the framebuffer
initializer to pass a desired pixel format instead of just
BPP as today, but that is a bit daunting, and Daniel said that
we would probably anyways need a fallback like this.
---
 drivers/gpu/drm/drm_fb_helper.c | 53 ++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d3af098b0922..e29432e486f4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1797,6 +1797,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	int i;
 	struct drm_fb_helper_surface_size sizes;
 	int gamma_size = 0;
+	int best_depth = 0;
 
 	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
 	sizes.surface_depth = 24;
@@ -1804,7 +1805,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	sizes.fb_width = (u32)-1;
 	sizes.fb_height = (u32)-1;
 
-	/* if driver picks 8 or 16 by default use that for both depth/bpp */
+	/*
+	 * If driver picks 8 or 16 by default use that for both depth/bpp
+	 * to begin with
+	 */
 	if (preferred_bpp != sizes.surface_bpp)
 		sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
 
@@ -1839,6 +1843,53 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		}
 	}
 
+	/*
+	 * If we run into a situation where, for example, the primary plane
+	 * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
+	 * 16) we need to scale down the depth of the sizes we request.
+	 */
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
+		struct drm_crtc *crtc = mode_set->crtc;
+		struct drm_plane *plane = crtc->primary;
+		int j;
+
+		DRM_DEBUG("test CRTC %d primary plane\n", i);
+
+		for (j = 0; j < plane->format_count; j++) {
+			const struct drm_format_info *fmt;
+
+			fmt = drm_format_info(plane->format_types[j]);
+
+			/*
+			 * Do not consider YUV or other complicated formats
+			 * for framebuffers. This means only legacy formats
+			 * are supported (fmt->depth is a legacy field) but
+			 * the framebuffer emulation can only deal with such
+			 * formats, specifically RGB/BGA formats.
+			 */
+			if (fmt->depth = 0)
+				continue;
+
+			/* We found a perfect fit, great */
+			if (fmt->depth = sizes.surface_depth)
+				break;
+
+			/* Skip depths above what we're looking for */
+			if (fmt->depth > sizes.surface_depth)
+				continue;
+
+			/* Best depth found so far */
+			if (fmt->depth > best_depth)
+				best_depth = fmt->depth;
+		}
+	}
+	if (sizes.surface_depth != best_depth) {
+		DRM_INFO("requested bpp %d, scaled depth down to %d",
+			  sizes.surface_bpp, best_depth);
+		sizes.surface_depth = best_depth;
+	}
+
 	crtc_count = 0;
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_display_mode *desired_mode;
-- 
2.19.2

WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij@linaro.org>
To: "Bartlomiej Zolnierkiewicz" <b.zolnierkie@samsung.com>,
	"Eric Anholt" <eric@anholt.net>,
	"Noralf Trønnes" <noralf@tronnes.org>,
	"Dave Airlie" <airlied@redhat.com>,
	"David Lechner" <david@lechnology.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	linux-fbdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org,
	"Linus Walleij" <linus.walleij@linaro.org>
Subject: [PATCH v3] drm/fb-helper: Scale back depth to supported maximum
Date: Thu, 10 Jan 2019 10:24:03 +0100	[thread overview]
Message-ID: <20190110092403.28324-1-linus.walleij@linaro.org> (raw)

The following happened when migrating an old fbdev driver to DRM:

The Integrator/CP PL111 supports 16BPP but only ARGB1555/ABGR1555
or XRGB1555/XBGR1555 i.e. the maximum depth is 15.

This makes the initialization of the framebuffer fail since
the code in drm_fb_helper_single_fb_probe() assigns the same value
to sizes.surface_bpp and sizes.surface_depth. I.e. it simply assumes
a 1-to-1 mapping between BPP and depth, which is true in most cases
but not for this hardware that only support odd formats.

To support the odd case of a driver supporting 16BPP with only 15
bits of depth, this patch will make the code loop over the formats
supported on the primary plane on each CRTC managed by the FB
helper and cap the depth to the maximum supported on any primary
plane.

On the PL110 Integrator, this makes drm_mode_legacy_fb_format()
select DRM_FORMAT_XRGB1555 which is acceptable for this driver, and
thus we get framebuffer, penguin and console on the Integrator/CP.

Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Rebased and tested with v5.0-rc1
- Skip any formats with fmt->depth == 0
- Collected Daniels review tag.
ChangeLog v1->v2:
- Loop over the CRTCs managed by the helper and check the
  crtc->primary on each CRTC for the applicable formats and
  thus depths.
- Skip over YUV formats. The framebuffer emulation cannot
  handle these formats.

The v1 was sent some while back in february and I only recently
got back to fixing this up to support the last CLCD displays.
It was agreed that it is probably best to augment the framebuffer
initializer to pass a desired pixel format instead of just
BPP as today, but that is a bit daunting, and Daniel said that
we would probably anyways need a fallback like this.
---
 drivers/gpu/drm/drm_fb_helper.c | 53 ++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d3af098b0922..e29432e486f4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1797,6 +1797,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	int i;
 	struct drm_fb_helper_surface_size sizes;
 	int gamma_size = 0;
+	int best_depth = 0;
 
 	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
 	sizes.surface_depth = 24;
@@ -1804,7 +1805,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	sizes.fb_width = (u32)-1;
 	sizes.fb_height = (u32)-1;
 
-	/* if driver picks 8 or 16 by default use that for both depth/bpp */
+	/*
+	 * If driver picks 8 or 16 by default use that for both depth/bpp
+	 * to begin with
+	 */
 	if (preferred_bpp != sizes.surface_bpp)
 		sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
 
@@ -1839,6 +1843,53 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		}
 	}
 
+	/*
+	 * If we run into a situation where, for example, the primary plane
+	 * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
+	 * 16) we need to scale down the depth of the sizes we request.
+	 */
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
+		struct drm_crtc *crtc = mode_set->crtc;
+		struct drm_plane *plane = crtc->primary;
+		int j;
+
+		DRM_DEBUG("test CRTC %d primary plane\n", i);
+
+		for (j = 0; j < plane->format_count; j++) {
+			const struct drm_format_info *fmt;
+
+			fmt = drm_format_info(plane->format_types[j]);
+
+			/*
+			 * Do not consider YUV or other complicated formats
+			 * for framebuffers. This means only legacy formats
+			 * are supported (fmt->depth is a legacy field) but
+			 * the framebuffer emulation can only deal with such
+			 * formats, specifically RGB/BGA formats.
+			 */
+			if (fmt->depth == 0)
+				continue;
+
+			/* We found a perfect fit, great */
+			if (fmt->depth == sizes.surface_depth)
+				break;
+
+			/* Skip depths above what we're looking for */
+			if (fmt->depth > sizes.surface_depth)
+				continue;
+
+			/* Best depth found so far */
+			if (fmt->depth > best_depth)
+				best_depth = fmt->depth;
+		}
+	}
+	if (sizes.surface_depth != best_depth) {
+		DRM_INFO("requested bpp %d, scaled depth down to %d",
+			  sizes.surface_bpp, best_depth);
+		sizes.surface_depth = best_depth;
+	}
+
 	crtc_count = 0;
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_display_mode *desired_mode;
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij@linaro.org>
To: "Bartlomiej Zolnierkiewicz" <b.zolnierkie@samsung.com>,
	"Eric Anholt" <eric@anholt.net>,
	"Noralf Trønnes" <noralf@tronnes.org>,
	"Dave Airlie" <airlied@redhat.com>,
	"David Lechner" <david@lechnology.com>
Cc: linux-fbdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org
Subject: [PATCH v3] drm/fb-helper: Scale back depth to supported maximum
Date: Thu, 10 Jan 2019 10:24:03 +0100	[thread overview]
Message-ID: <20190110092403.28324-1-linus.walleij@linaro.org> (raw)

The following happened when migrating an old fbdev driver to DRM:

The Integrator/CP PL111 supports 16BPP but only ARGB1555/ABGR1555
or XRGB1555/XBGR1555 i.e. the maximum depth is 15.

This makes the initialization of the framebuffer fail since
the code in drm_fb_helper_single_fb_probe() assigns the same value
to sizes.surface_bpp and sizes.surface_depth. I.e. it simply assumes
a 1-to-1 mapping between BPP and depth, which is true in most cases
but not for this hardware that only support odd formats.

To support the odd case of a driver supporting 16BPP with only 15
bits of depth, this patch will make the code loop over the formats
supported on the primary plane on each CRTC managed by the FB
helper and cap the depth to the maximum supported on any primary
plane.

On the PL110 Integrator, this makes drm_mode_legacy_fb_format()
select DRM_FORMAT_XRGB1555 which is acceptable for this driver, and
thus we get framebuffer, penguin and console on the Integrator/CP.

Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Rebased and tested with v5.0-rc1
- Skip any formats with fmt->depth == 0
- Collected Daniels review tag.
ChangeLog v1->v2:
- Loop over the CRTCs managed by the helper and check the
  crtc->primary on each CRTC for the applicable formats and
  thus depths.
- Skip over YUV formats. The framebuffer emulation cannot
  handle these formats.

The v1 was sent some while back in february and I only recently
got back to fixing this up to support the last CLCD displays.
It was agreed that it is probably best to augment the framebuffer
initializer to pass a desired pixel format instead of just
BPP as today, but that is a bit daunting, and Daniel said that
we would probably anyways need a fallback like this.
---
 drivers/gpu/drm/drm_fb_helper.c | 53 ++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d3af098b0922..e29432e486f4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1797,6 +1797,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	int i;
 	struct drm_fb_helper_surface_size sizes;
 	int gamma_size = 0;
+	int best_depth = 0;
 
 	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
 	sizes.surface_depth = 24;
@@ -1804,7 +1805,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	sizes.fb_width = (u32)-1;
 	sizes.fb_height = (u32)-1;
 
-	/* if driver picks 8 or 16 by default use that for both depth/bpp */
+	/*
+	 * If driver picks 8 or 16 by default use that for both depth/bpp
+	 * to begin with
+	 */
 	if (preferred_bpp != sizes.surface_bpp)
 		sizes.surface_depth = sizes.surface_bpp = preferred_bpp;
 
@@ -1839,6 +1843,53 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		}
 	}
 
+	/*
+	 * If we run into a situation where, for example, the primary plane
+	 * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
+	 * 16) we need to scale down the depth of the sizes we request.
+	 */
+	for (i = 0; i < fb_helper->crtc_count; i++) {
+		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
+		struct drm_crtc *crtc = mode_set->crtc;
+		struct drm_plane *plane = crtc->primary;
+		int j;
+
+		DRM_DEBUG("test CRTC %d primary plane\n", i);
+
+		for (j = 0; j < plane->format_count; j++) {
+			const struct drm_format_info *fmt;
+
+			fmt = drm_format_info(plane->format_types[j]);
+
+			/*
+			 * Do not consider YUV or other complicated formats
+			 * for framebuffers. This means only legacy formats
+			 * are supported (fmt->depth is a legacy field) but
+			 * the framebuffer emulation can only deal with such
+			 * formats, specifically RGB/BGA formats.
+			 */
+			if (fmt->depth == 0)
+				continue;
+
+			/* We found a perfect fit, great */
+			if (fmt->depth == sizes.surface_depth)
+				break;
+
+			/* Skip depths above what we're looking for */
+			if (fmt->depth > sizes.surface_depth)
+				continue;
+
+			/* Best depth found so far */
+			if (fmt->depth > best_depth)
+				best_depth = fmt->depth;
+		}
+	}
+	if (sizes.surface_depth != best_depth) {
+		DRM_INFO("requested bpp %d, scaled depth down to %d",
+			  sizes.surface_bpp, best_depth);
+		sizes.surface_depth = best_depth;
+	}
+
 	crtc_count = 0;
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_display_mode *desired_mode;
-- 
2.19.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

             reply	other threads:[~2019-01-10  9:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-10  9:24 Linus Walleij [this message]
2019-01-10  9:24 ` [PATCH v3] drm/fb-helper: Scale back depth to supported maximum Linus Walleij
2019-01-10  9:24 ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190110092403.28324-1-linus.walleij@linaro.org \
    --to=linus.walleij@linaro.org \
    --cc=airlied@redhat.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=david@lechnology.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eric@anholt.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=noralf@tronnes.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.