On 4/27/22 04:43, Pekka Paalanen wrote:
On Tue, 26 Apr 2022 22:22:22 -0300
Igor Torrente <igormtorrente@xxxxxxxxx> wrote:
On April 26, 2022 10:03:09 PM GMT-03:00, Igor Torrente <igormtorrente@xxxxxxxxx> wrote:
On 4/25/22 22:54, Igor Torrente wrote:
Hi Pekka,
On 4/25/22 05:10, Pekka Paalanen wrote:
On Sat, 23 Apr 2022 15:53:20 -0300
Igor Torrente <igormtorrente@xxxxxxxxx> wrote:
...
+static void argb_u16_to_XRGB8888(struct vkms_frame_info *frame_info,
+ const struct line_buffer *src_buffer, int y)
+{
+ int x, x_dst = frame_info->dst.x1;
+ u8 *dst_pixels = packed_pixels_addr(frame_info, x_dst, y);
+ struct pixel_argb_u16 *in_pixels = src_buffer->pixels;
+ int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst),
+ src_buffer->n_pixels);
+
+ for (x = 0; x < x_limit; x++, dst_pixels += 4) {
+ dst_pixels[3] = (u8)0xff;
When writing to XRGB, it's not necessary to ensure the X channel has
any sensible value. Anyone reading from XRGB must ignore that value
anyway. So why not write something wacky here, like 0xa1, that is far
enough from both 0x00 or 0xff to not be confused with them even
visually? Also not 0x7f or 0x80 which are close to half of 0xff.
Or, you could save a whole function and just use argb_u16_to_ARGBxxxx()
instead, even for XRGB destination.
Right. Maybe I could just leave the channel untouched.
Untouched may not be a good idea. Leaving anything untouched always has
the risk of leaking information through uninitialized memory. Maybe not
in this case because the destination is allocated by userspace already,
but nothing beats being obviously correct.
Makes sense.
Whatever you decide here, be prepared for it becoming de-facto kernel
UABI, because it is easy for userspace to (accidentally) rely on the
value, no matter what you pick.
I hope to make the right decision then.
The de-facto UABI seems to be already in place for {A, X}RGB8888.
"Only XRGB_8888
If that's only IGT, then you should raise an issue with IGT about this,
to figure out if IGT is wrong by accident or if it is deliberate, and
are we stuck with it.
This is why I would want to fill X with garbage, to make the
expectations clear before the "obvious and logical constant value for X"
makes a mess by making XRGB indistinguishable from ARGB. Then the next
question is, do we need a special function to write out XRGB values, or
can we simply re-use the ARGB function.
Do the tests expect X channel to be filled with 0xff or with the actual
A values? This question will matter when all planes have ARGB
framebuffers and no background color. Then even more questions will
arise about what should actually happen with A values (blending
equation).
I dig into the igt code a little bit and found that it's waiting for the
channel to not be changed.
It fills all the pixels in the line with a value and calculates the CRC
of the entire buffer, including the alpha.
I will crate an issue asking if this is intended.
I changed from 0xff to 0xbe and the `writeback-check-output` started to fail.
Thanks,
pq
