Re: [PATCH 2/7] drm/logicvc: Fix preferred fbdev cpp
On 11/18/22 14:48, Thomas Zimmermann wrote: [...] >>> >> >> Ah, I see. So is to set 32-bit bpp for both XRGB and ARGB. But then >> I think that you also need to fix logicvc_mode_init() to remove that += 8? >> >> Because right now the preferred_depth += 8 would set a preferred_depth of 32 >> when should be just 24 even if alpha is enabled? >> >> Or am I confusing again the meaning of the color depth? > > For DRM, it's defined in drm_fourcc.c. ARGB has a depth of 32 and > XRGB has a depth of 24. Both have a bpp of 32. > > BUT in logicvc's internal data structure, both formats have a color > depth of 24 and a bpp of 32. > Got it. Thanks for the explanations and sorry for the silly questions then. If you drop that 15-bit preferred depth case, feel free to add Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 2/7] drm/logicvc: Fix preferred fbdev cpp
Hi Am 18.11.22 um 14:41 schrieb Javier Martinez Canillas: On 11/18/22 14:22, Thomas Zimmermann wrote: [...] I'm also not sure if this is needed. Since IIUC in logicvc_mode_init() the driver does: preferred_depth = layer_primary->formats->depth; /* DRM counts alpha in depth, our driver doesn't. */ if (layer_primary->formats->alpha) preferred_depth += 8; ... mode_config->preferred_depth = preferred_depth; So it seems this patch is not needed? Unless I'm misunderstanding the code. The driver uses XRGB, so the 24-bit color depth has a 32-bit bpp value. Hence the switch. Ah, I see. So is to set 32-bit bpp for both XRGB and ARGB. But then I think that you also need to fix logicvc_mode_init() to remove that += 8? Because right now the preferred_depth += 8 would set a preferred_depth of 32 when should be just 24 even if alpha is enabled? Or am I confusing again the meaning of the color depth? For DRM, it's defined in drm_fourcc.c. ARGB has a depth of 32 and XRGB has a depth of 24. Both have a bpp of 32. BUT in logicvc's internal data structure, both formats have a color depth of 24 and a bpp of 32. Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/7] drm/logicvc: Fix preferred fbdev cpp
On 11/18/22 14:22, Thomas Zimmermann wrote: [...] >> >> I'm also not sure if this is needed. Since IIUC in logicvc_mode_init() the >> driver does: >> >> preferred_depth = layer_primary->formats->depth; >> >> /* DRM counts alpha in depth, our driver doesn't. */ >> if (layer_primary->formats->alpha) >> preferred_depth += 8; >> >> ... >> mode_config->preferred_depth = preferred_depth; >> >> So it seems this patch is not needed? Unless I'm misunderstanding the code. > > The driver uses XRGB, so the 24-bit color depth has a 32-bit bpp > value. Hence the switch. > Ah, I see. So is to set 32-bit bpp for both XRGB and ARGB. But then I think that you also need to fix logicvc_mode_init() to remove that += 8? Because right now the preferred_depth += 8 would set a preferred_depth of 32 when should be just 24 even if alpha is enabled? Or am I confusing again the meaning of the color depth? -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH 2/7] drm/logicvc: Fix preferred fbdev cpp
Hi
Am 18.11.22 um 14:08 schrieb Javier Martinez Canillas:
On 11/16/22 17:09, Thomas Zimmermann wrote:
Logicvc can have different values for the preferred color depth. Set
the fbdev bpp value depending on the runtime value.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/logicvc/logicvc_drm.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c
b/drivers/gpu/drm/logicvc/logicvc_drm.c
index 9de24d9f0c963..d9cd5d967e31f 100644
--- a/drivers/gpu/drm/logicvc/logicvc_drm.c
+++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
@@ -301,6 +301,7 @@ static int logicvc_drm_probe(struct platform_device *pdev)
struct regmap *regmap = NULL;
struct resource res;
void __iomem *base;
+ unsigned int preferred_bpp;
int irq;
int ret;
@@ -438,7 +439,18 @@ static int logicvc_drm_probe(struct platform_device *pdev)
goto error_mode;
}
- drm_fbdev_generic_setup(drm_dev, drm_dev->mode_config.preferred_depth);
+ switch (drm_dev->mode_config.preferred_depth) {
+ case 15:
Why could have 15? IIUC the formats supported by this driver are:
static uint32_t logicvc_layer_formats_rgb16[] = {
DRM_FORMAT_RGB565,
DRM_FORMAT_BGR565,
DRM_FORMAT_INVALID,
};
static uint32_t logicvc_layer_formats_rgb24[] = {
DRM_FORMAT_XRGB,
DRM_FORMAT_XBGR,
DRM_FORMAT_INVALID,
};
/*
* What we call depth in this driver only counts color components, not alpha.
* This allows us to stay compatible with the LogiCVC bistream definitions.
*/
static uint32_t logicvc_layer_formats_rgb24_alpha[] = {
DRM_FORMAT_ARGB,
DRM_FORMAT_ABGR,
DRM_FORMAT_INVALID,
};
So shouldn't be just 16, 24 and 32 ?
That makes sense.
+ case 16:
+ preferred_bpp = 16;
+ break;
+ case 24:
+ case 32:
+ default:
+ preferred_bpp = 32;
+ break;
I'm also not sure if this is needed. Since IIUC in logicvc_mode_init() the
driver does:
preferred_depth = layer_primary->formats->depth;
/* DRM counts alpha in depth, our driver doesn't. */
if (layer_primary->formats->alpha)
preferred_depth += 8;
...
mode_config->preferred_depth = preferred_depth;
So it seems this patch is not needed? Unless I'm misunderstanding the code.
The driver uses XRGB, so the 24-bit color depth has a 32-bit bpp
value. Hence the switch.
Best regards
Thomas
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH 2/7] drm/logicvc: Fix preferred fbdev cpp
On 11/16/22 17:09, Thomas Zimmermann wrote:
> Logicvc can have different values for the preferred color depth. Set
> the fbdev bpp value depending on the runtime value.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/logicvc/logicvc_drm.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c
> b/drivers/gpu/drm/logicvc/logicvc_drm.c
> index 9de24d9f0c963..d9cd5d967e31f 100644
> --- a/drivers/gpu/drm/logicvc/logicvc_drm.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
> @@ -301,6 +301,7 @@ static int logicvc_drm_probe(struct platform_device *pdev)
> struct regmap *regmap = NULL;
> struct resource res;
> void __iomem *base;
> + unsigned int preferred_bpp;
> int irq;
> int ret;
>
> @@ -438,7 +439,18 @@ static int logicvc_drm_probe(struct platform_device
> *pdev)
> goto error_mode;
> }
>
> - drm_fbdev_generic_setup(drm_dev, drm_dev->mode_config.preferred_depth);
> + switch (drm_dev->mode_config.preferred_depth) {
> + case 15:
Why could have 15? IIUC the formats supported by this driver are:
static uint32_t logicvc_layer_formats_rgb16[] = {
DRM_FORMAT_RGB565,
DRM_FORMAT_BGR565,
DRM_FORMAT_INVALID,
};
static uint32_t logicvc_layer_formats_rgb24[] = {
DRM_FORMAT_XRGB,
DRM_FORMAT_XBGR,
DRM_FORMAT_INVALID,
};
/*
* What we call depth in this driver only counts color components, not alpha.
* This allows us to stay compatible with the LogiCVC bistream definitions.
*/
static uint32_t logicvc_layer_formats_rgb24_alpha[] = {
DRM_FORMAT_ARGB,
DRM_FORMAT_ABGR,
DRM_FORMAT_INVALID,
};
So shouldn't be just 16, 24 and 32 ?
> + case 16:
> + preferred_bpp = 16;
> + break;
> + case 24:
> + case 32:
> + default:
> + preferred_bpp = 32;
> + break;
I'm also not sure if this is needed. Since IIUC in logicvc_mode_init() the
driver does:
preferred_depth = layer_primary->formats->depth;
/* DRM counts alpha in depth, our driver doesn't. */
if (layer_primary->formats->alpha)
preferred_depth += 8;
...
mode_config->preferred_depth = preferred_depth;
So it seems this patch is not needed? Unless I'm misunderstanding the code.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
[PATCH 2/7] drm/logicvc: Fix preferred fbdev cpp
Logicvc can have different values for the preferred color depth. Set
the fbdev bpp value depending on the runtime value.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/logicvc/logicvc_drm.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c
b/drivers/gpu/drm/logicvc/logicvc_drm.c
index 9de24d9f0c963..d9cd5d967e31f 100644
--- a/drivers/gpu/drm/logicvc/logicvc_drm.c
+++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
@@ -301,6 +301,7 @@ static int logicvc_drm_probe(struct platform_device *pdev)
struct regmap *regmap = NULL;
struct resource res;
void __iomem *base;
+ unsigned int preferred_bpp;
int irq;
int ret;
@@ -438,7 +439,18 @@ static int logicvc_drm_probe(struct platform_device *pdev)
goto error_mode;
}
- drm_fbdev_generic_setup(drm_dev, drm_dev->mode_config.preferred_depth);
+ switch (drm_dev->mode_config.preferred_depth) {
+ case 15:
+ case 16:
+ preferred_bpp = 16;
+ break;
+ case 24:
+ case 32:
+ default:
+ preferred_bpp = 32;
+ break;
+ }
+ drm_fbdev_generic_setup(drm_dev, preferred_bpp);
return 0;
--
2.38.1
