drm/vc4: plane: Expand the lower bits using the LSB
Subsystems (3)
PATCH 1/1 drm/vc4: plane: Expand the lower bits using the LSB
The vc4 HVS uses an internal RGB888 representation of the frames, and will by default expand formats using a lower depth using zeros.
This causes an issue when we try to use other compositing software such as pixman that seems to be filling the missing bits using the format least significant bit value. As such, this prevents us from checking the display output in a reliable way.
To prevent this, force the same behaviour so that we can do such things.
--- drivers/gpu/drm/vc4/vc4_plane.c | 1 + drivers/gpu/drm/vc4/vc4_regs.h | 8 ++++++++ 2 files changed, 9 insertions(+)
drivers/gpu/drm/vc4/vc4_plane.c +1 -0
@@ -542,6 +542,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane,/* Control word */ vc4_dlist_write(vc4_state, SCALER_CTL0_VALID | + VC4_SET_FIELD(SCALER_CTL0_EXPAND_LSB, SCALER_CTL0_EXPAND) | (format->pixel_order << SCALER_CTL0_ORDER_SHIFT) | (format->hvs << SCALER_CTL0_PIXEL_FORMAT_SHIFT) | VC4_SET_FIELD(tiling, SCALER_CTL0_TILING) |
drivers/gpu/drm/vc4/vc4_regs.h +8 -0
@@ -806,12 +806,20 @@ enum hvs_pixel_format {#define SCALER_CTL0_ORDER_MASK VC4_MASK(14, 13)#define SCALER_CTL0_ORDER_SHIFT 13 +#define SCALER_CTL0_EXPAND_MASK VC4_MASK(12, 11) +#define SCALER_CTL0_EXPAND_SHIFT 11 +#define SCALER_CTL0_SCL1_MASK VC4_MASK(10, 8)#define SCALER_CTL0_SCL1_SHIFT 8#define SCALER_CTL0_SCL0_MASK VC4_MASK(7, 5)#define SCALER_CTL0_SCL0_SHIFT 5 +#define SCALER_CTL0_EXPAND_ZERO 0 +#define SCALER_CTL0_EXPAND_LSB 1 +#define SCALER_CTL0_EXPAND_MSB 2 +#define SCALER_CTL0_EXPAND_REPEAT 3 +#define SCALER_CTL0_SCL_H_PPF_V_PPF 0#define SCALER_CTL0_SCL_H_TPZ_V_PPF 1#define SCALER_CTL0_SCL_H_PPF_V_TPZ 2
Signature · 6 lines
2.17.0 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 7 lines · Maxime Ripard
The vc4 HVS uses an internal RGB888 representation of the frames, and will by default expand formats using a lower depth using zeros.
This causes an issue when we try to use other compositing software such as pixman that seems to be filling the missing bits using the format least significant bit value. As such, this prevents us from checking the display output in a reliable way.
I don't think this is the right expansion function, though? My understanding of proper unorm expansion, and what pixman's unorm_to_unorm() does, is that you replicate the value in the missing bits until you've filled them all (so SCALER_CTL0_EXPAND_REPEAT)
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Eric,
Quoted 10 lines · Eric Anholt
Quoted 7 lines · Maxime Ripard
The vc4 HVS uses an internal RGB888 representation of the frames, and will by default expand formats using a lower depth using zeros.
This causes an issue when we try to use other compositing software such as pixman that seems to be filling the missing bits using the format least significant bit value. As such, this prevents us from checking the display output in a reliable way.
Quoted 4 lines
I don't think this is the right expansion function, though? My understanding of proper unorm expansion, and what pixman's
unorm_to_unorm()does, is that you replicate the value in the missing bits until you've filled them all (soSCALER_CTL0_EXPAND_REPEAT)
This was based on experiment, so I might have missed that case and you are probably right. I'll test with SCALER_CTL0_EXPAND_REPEAT and let you know :)
Thanks!
Maxime
Signature · 3 lines
Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 39 lines · Maxime Ripard
The vc4 HVS uses an internal RGB888 representation of the frames, and will by default expand formats using a lower depth using zeros.
This causes an issue when we try to use other compositing software such as pixman that seems to be filling the missing bits using the format least significant bit value. As such, this prevents us from checking the display output in a reliable way.
To prevent this, force the same behaviour so that we can do such things.
Signed-off-by Maxime Ripard <[email protected]>--- drivers/gpu/drm/vc4/vc4_plane.c | 1 + drivers/gpu/drm/vc4/vc4_regs.h | 8 ++++++++ 2 files changed, 9 insertions(+)drivers/gpu/drm/vc4/vc4_plane.c +1
-0@@ -542,6 +542,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane,/* Control word */ vc4_dlist_write(vc4_state, SCALER_CTL0_VALID | + VC4_SET_FIELD(SCALER_CTL0_EXPAND_LSB, SCALER_CTL0_EXPAND) | (format->pixel_order << SCALER_CTL0_ORDER_SHIFT) | (format->hvs << SCALER_CTL0_PIXEL_FORMAT_SHIFT) | VC4_SET_FIELD(tiling, SCALER_CTL0_TILING) |drivers/gpu/drm/vc4/vc4_regs.h +3
-0@@ -806,12 +806,20 @@ enum hvs_pixel_format {#define SCALER_CTL0_ORDER_MASK VC4_MASK(14, 13)#define SCALER_CTL0_ORDER_SHIFT 13 +#define SCALER_CTL0_EXPAND_MASK VC4_MASK(12, 11) +#define SCALER_CTL0_EXPAND_SHIFT 11 +
drivers/gpu/drm/vc4/vc4_regs.h enum hvs_pixel_format { · line 821
Quoted 10 lines
drivers/gpu/drm/vc4/vc4_regs.h +4
-0#define SCALER_CTL0_SCL1_MASK VC4_MASK(10, 8)#define SCALER_CTL0_SCL1_SHIFT 8#define SCALER_CTL0_SCL0_MASK VC4_MASK(7, 5)#define SCALER_CTL0_SCL0_SHIFT 5 +#define SCALER_CTL0_EXPAND_ZERO 0 +#define SCALER_CTL0_EXPAND_LSB 1 +#define SCALER_CTL0_EXPAND_MSB 2 +#define SCALER_CTL0_EXPAND_REPEAT 3
Just a heads up, those defines have recently landed in drm-misc, with slightly different naming: SCALER_CTL0_RGBA_EXPAND_*
Thanks,
Stefan
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel