| Message ID | 20230728200714.2084223-1-dhobsong@igel.co.jp (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 0dfcf80d41a20d83e41b63dab11eb17d3de69503 |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | [1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support | expand |
Hi Damian, Thank you for the patch. On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote: > Add additional pixel formats for which blending is disabling when Did you mean "disabled" instead of "disabling" ? > DRM_MODE_BLEND_PIXEL_NONE is set. > > Refactor the fourcc selection into a separate function to handle the > increased number of formats. > > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> > --- > drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- > 1 file changed, 32 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > index 45c05d0ffc70..96241c03b60f 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { > DRM_FORMAT_Y212, > }; > > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) > +{ > + u32 fourcc = state->format->fourcc; > + > + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > + switch (fourcc) { > + case DRM_FORMAT_ARGB1555: > + fourcc = DRM_FORMAT_XRGB1555; > + break; > + > + case DRM_FORMAT_ARGB4444: > + fourcc = DRM_FORMAT_XRGB4444; > + break; > + > + case DRM_FORMAT_ARGB8888: > + fourcc = DRM_FORMAT_XRGB8888; > + break; > + > + case DRM_FORMAT_BGRA8888: > + fourcc = DRM_FORMAT_BGRX8888; > + break; > + > + case DRM_FORMAT_RGBA1010102: > + fourcc = DRM_FORMAT_RGBX1010102; > + break; Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out intentionally ? > + } > + } > + > + return fourcc; > +} > + > static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > { > struct rcar_du_vsp_plane_state *state = > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > .alpha = state->state.alpha >> 8, > .zpos = state->state.zpos, > }; > - u32 fourcc = state->format->fourcc; > + u32 fourcc = rcar_du_vsp_state_get_format(state); > unsigned int i; > > cfg.src.left = state->state.src.x1 >> 16; > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl) > + fb->offsets[i]; > > - if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > - switch (fourcc) { > - case DRM_FORMAT_ARGB1555: > - fourcc = DRM_FORMAT_XRGB1555; > - break; > - > - case DRM_FORMAT_ARGB4444: > - fourcc = DRM_FORMAT_XRGB4444; > - break; > - > - case DRM_FORMAT_ARGB8888: > - fourcc = DRM_FORMAT_XRGB8888; > - break; > - } > - } > - > format = rcar_du_format_info(fourcc); > cfg.pixelformat = format->v4l2; >
On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote: > Hi Damian, > > Thank you for the patch. > > On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote: > > Add additional pixel formats for which blending is disabling when > > Did you mean "disabled" instead of "disabling" ? > > > DRM_MODE_BLEND_PIXEL_NONE is set. > > > > Refactor the fourcc selection into a separate function to handle the > > increased number of formats. > > > > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> > > --- > > drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- > > 1 file changed, 32 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > index 45c05d0ffc70..96241c03b60f 100644 > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { > > DRM_FORMAT_Y212, > > }; > > > > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) > > +{ > > + u32 fourcc = state->format->fourcc; > > + > > + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > > + switch (fourcc) { > > + case DRM_FORMAT_ARGB1555: > > + fourcc = DRM_FORMAT_XRGB1555; > > + break; > > + > > + case DRM_FORMAT_ARGB4444: > > + fourcc = DRM_FORMAT_XRGB4444; > > + break; > > + > > + case DRM_FORMAT_ARGB8888: > > + fourcc = DRM_FORMAT_XRGB8888; > > + break; > > + > > + case DRM_FORMAT_BGRA8888: > > + fourcc = DRM_FORMAT_BGRX8888; > > + break; > > + > > + case DRM_FORMAT_RGBA1010102: > > + fourcc = DRM_FORMAT_RGBX1010102; > > + break; > > Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out > intentionally ? It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment. Let's do so with a patch on top of this series. There's no need to send a v2, I can handle the simple change in the commit message if you let me know whether my comment is right or wrong. > > + } > > + } > > + > > + return fourcc; > > +} > > + > > static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > > { > > struct rcar_du_vsp_plane_state *state = > > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > > .alpha = state->state.alpha >> 8, > > .zpos = state->state.zpos, > > }; > > - u32 fourcc = state->format->fourcc; > > + u32 fourcc = rcar_du_vsp_state_get_format(state); > > unsigned int i; > > > > cfg.src.left = state->state.src.x1 >> 16; > > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > > cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl) > > + fb->offsets[i]; > > > > - if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > > - switch (fourcc) { > > - case DRM_FORMAT_ARGB1555: > > - fourcc = DRM_FORMAT_XRGB1555; > > - break; > > - > > - case DRM_FORMAT_ARGB4444: > > - fourcc = DRM_FORMAT_XRGB4444; > > - break; > > - > > - case DRM_FORMAT_ARGB8888: > > - fourcc = DRM_FORMAT_XRGB8888; > > - break; > > - } > > - } > > - > > format = rcar_du_format_info(fourcc); > > cfg.pixelformat = format->v4l2; > >
On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote: > On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote: > > Hi Damian, > > > > Thank you for the patch. > > > > On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote: > > > Add additional pixel formats for which blending is disabling when > > > > Did you mean "disabled" instead of "disabling" ? > > > > > DRM_MODE_BLEND_PIXEL_NONE is set. > > > > > > Refactor the fourcc selection into a separate function to handle the > > > increased number of formats. > > > > > > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> > > > --- > > > drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- > > > 1 file changed, 32 insertions(+), 17 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > > index 45c05d0ffc70..96241c03b60f 100644 > > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { > > > DRM_FORMAT_Y212, > > > }; > > > > > > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) > > > +{ > > > + u32 fourcc = state->format->fourcc; > > > + > > > + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > > > + switch (fourcc) { > > > + case DRM_FORMAT_ARGB1555: > > > + fourcc = DRM_FORMAT_XRGB1555; > > > + break; > > > + > > > + case DRM_FORMAT_ARGB4444: > > > + fourcc = DRM_FORMAT_XRGB4444; > > > + break; > > > + > > > + case DRM_FORMAT_ARGB8888: > > > + fourcc = DRM_FORMAT_XRGB8888; > > > + break; > > > + > > > + case DRM_FORMAT_BGRA8888: > > > + fourcc = DRM_FORMAT_BGRX8888; > > > + break; > > > + > > > + case DRM_FORMAT_RGBA1010102: > > > + fourcc = DRM_FORMAT_RGBX1010102; > > > + break; > > > > Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out > > intentionally ? > > It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as > DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment. > Let's do so with a patch on top of this series. Replying to myself again, the datasheet doesn't explicitly list DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to specify the location of the components should work fine for that format. Is this something you would be able to test ? > There's no need to send > a v2, I can handle the simple change in the commit message if you let me > know whether my comment is right or wrong. > > > > + } > > > + } > > > + > > > + return fourcc; > > > +} > > > + > > > static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > > > { > > > struct rcar_du_vsp_plane_state *state = > > > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > > > .alpha = state->state.alpha >> 8, > > > .zpos = state->state.zpos, > > > }; > > > - u32 fourcc = state->format->fourcc; > > > + u32 fourcc = rcar_du_vsp_state_get_format(state); > > > unsigned int i; > > > > > > cfg.src.left = state->state.src.x1 >> 16; > > > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) > > > cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl) > > > + fb->offsets[i]; > > > > > > - if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > > > - switch (fourcc) { > > > - case DRM_FORMAT_ARGB1555: > > > - fourcc = DRM_FORMAT_XRGB1555; > > > - break; > > > - > > > - case DRM_FORMAT_ARGB4444: > > > - fourcc = DRM_FORMAT_XRGB4444; > > > - break; > > > - > > > - case DRM_FORMAT_ARGB8888: > > > - fourcc = DRM_FORMAT_XRGB8888; > > > - break; > > > - } > > > - } > > > - > > > format = rcar_du_format_info(fourcc); > > > cfg.pixelformat = format->v4l2; > > >
Hi Laurent, Thank you for the review. On 2023/08/03 20:06, Laurent Pinchart wrote: > On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote: >> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote: >>> Hi Damian, >>> >>> Thank you for the patch. >>> >>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote: >>>> Add additional pixel formats for which blending is disabling when >>> Did you mean "disabled" instead of "disabling" ? Oops. Yes, that's exactly what I meant. >>> >>>> DRM_MODE_BLEND_PIXEL_NONE is set. >>>> >>>> Refactor the fourcc selection into a separate function to handle the >>>> increased number of formats. >>>> >>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> >>>> --- >>>> drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- >>>> 1 file changed, 32 insertions(+), 17 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c >>>> index 45c05d0ffc70..96241c03b60f 100644 >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c >>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { >>>> DRM_FORMAT_Y212, >>>> }; >>>> >>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) >>>> +{ >>>> + u32 fourcc = state->format->fourcc; >>>> + >>>> + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { >>>> + switch (fourcc) { >>>> + case DRM_FORMAT_ARGB1555: >>>> + fourcc = DRM_FORMAT_XRGB1555; >>>> + break; >>>> + >>>> + case DRM_FORMAT_ARGB4444: >>>> + fourcc = DRM_FORMAT_XRGB4444; >>>> + break; >>>> + >>>> + case DRM_FORMAT_ARGB8888: >>>> + fourcc = DRM_FORMAT_XRGB8888; >>>> + break; >>>> + >>>> + case DRM_FORMAT_BGRA8888: >>>> + fourcc = DRM_FORMAT_BGRX8888; >>>> + break; >>>> + >>>> + case DRM_FORMAT_RGBA1010102: >>>> + fourcc = DRM_FORMAT_RGBX1010102; >>>> + break; >>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out >>> intentionally ? Yes, it was intentionally left out for the time being for the reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not being handled by the DU driver). >> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as >> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment. >> Let's do so with a patch on top of this series. Yes, I was thinking the same thing. > Replying to myself again, the datasheet doesn't explicitly list > DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to > specify the location of the components should work fine for that format. > Is this something you would be able to test ? Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 formats with. I will double-check with the office in Japan, but I don't think that they have one either. > >> There's no need to send >> a v2, I can handle the simple change in the commit message if you let me >> know whether my comment is right or wrong. Thank you, Damian
Hi Damian, (CC'ing Tomi) On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote: > On 2023/08/03 20:06, Laurent Pinchart wrote: > > On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote: > >> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote: > >>> Hi Damian, > >>> > >>> Thank you for the patch. > >>> > >>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote: > >>>> Add additional pixel formats for which blending is disabling when > >>> Did you mean "disabled" instead of "disabling" ? > > Oops. Yes, that's exactly what I meant. > > >>> > >>>> DRM_MODE_BLEND_PIXEL_NONE is set. > >>>> > >>>> Refactor the fourcc selection into a separate function to handle the > >>>> increased number of formats. > >>>> > >>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> > >>>> --- > >>>> drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- > >>>> 1 file changed, 32 insertions(+), 17 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > >>>> index 45c05d0ffc70..96241c03b60f 100644 > >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > >>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { > >>>> DRM_FORMAT_Y212, > >>>> }; > >>>> > >>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) > >>>> +{ > >>>> + u32 fourcc = state->format->fourcc; > >>>> + > >>>> + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > >>>> + switch (fourcc) { > >>>> + case DRM_FORMAT_ARGB1555: > >>>> + fourcc = DRM_FORMAT_XRGB1555; > >>>> + break; > >>>> + > >>>> + case DRM_FORMAT_ARGB4444: > >>>> + fourcc = DRM_FORMAT_XRGB4444; > >>>> + break; > >>>> + > >>>> + case DRM_FORMAT_ARGB8888: > >>>> + fourcc = DRM_FORMAT_XRGB8888; > >>>> + break; > >>>> + > >>>> + case DRM_FORMAT_BGRA8888: > >>>> + fourcc = DRM_FORMAT_BGRX8888; > >>>> + break; > >>>> + > >>>> + case DRM_FORMAT_RGBA1010102: > >>>> + fourcc = DRM_FORMAT_RGBX1010102; > >>>> + break; > >>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out > >>> intentionally ? > > Yes, it was intentionally left out for the time being for the > reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not > being handled by the DU driver). > > >> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as > >> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment. > >> Let's do so with a patch on top of this series. > Yes, I was thinking the same thing. > > Replying to myself again, the datasheet doesn't explicitly list > > DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to > > specify the location of the components should work fine for that format. > > Is this something you would be able to test ? > > Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 > formats with. > I will double-check with the office in Japan, but I don't think that > they have one either. Tomi, is this something you could test ? > >> There's no need to send > >> a v2, I can handle the simple change in the commit message if you let me > >> know whether my comment is right or wrong.
On Fri, Aug 04, 2023 at 06:52:51PM +0300, Laurent Pinchart wrote: > On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote: > > On 2023/08/03 20:06, Laurent Pinchart wrote: > > > On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote: > > >> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote: > > >>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote: > > >>>> Add additional pixel formats for which blending is disabling when > > >>> > > >>> Did you mean "disabled" instead of "disabling" ? > > > > Oops. Yes, that's exactly what I meant. I'll fix this locally. > > >>>> DRM_MODE_BLEND_PIXEL_NONE is set. > > >>>> > > >>>> Refactor the fourcc selection into a separate function to handle the > > >>>> increased number of formats. > > >>>> > > >>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> > > >>>> --- > > >>>> drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- > > >>>> 1 file changed, 32 insertions(+), 17 deletions(-) > > >>>> > > >>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > >>>> index 45c05d0ffc70..96241c03b60f 100644 > > >>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > >>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c > > >>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { > > >>>> DRM_FORMAT_Y212, > > >>>> }; > > >>>> > > >>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) > > >>>> +{ > > >>>> + u32 fourcc = state->format->fourcc; > > >>>> + > > >>>> + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { > > >>>> + switch (fourcc) { > > >>>> + case DRM_FORMAT_ARGB1555: > > >>>> + fourcc = DRM_FORMAT_XRGB1555; > > >>>> + break; > > >>>> + > > >>>> + case DRM_FORMAT_ARGB4444: > > >>>> + fourcc = DRM_FORMAT_XRGB4444; > > >>>> + break; > > >>>> + > > >>>> + case DRM_FORMAT_ARGB8888: > > >>>> + fourcc = DRM_FORMAT_XRGB8888; > > >>>> + break; > > >>>> + > > >>>> + case DRM_FORMAT_BGRA8888: > > >>>> + fourcc = DRM_FORMAT_BGRX8888; > > >>>> + break; > > >>>> + > > >>>> + case DRM_FORMAT_RGBA1010102: > > >>>> + fourcc = DRM_FORMAT_RGBX1010102; > > >>>> + break; > > >>> > > >>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out > > >>> intentionally ? > > > > Yes, it was intentionally left out for the time being for the > > reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not > > being handled by the DU driver). > > > > >> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as > > >> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment. > > >> Let's do so with a patch on top of this series. > > > > Yes, I was thinking the same thing. > > > > > Replying to myself again, the datasheet doesn't explicitly list > > > DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to > > > specify the location of the components should work fine for that format. > > > Is this something you would be able to test ? > > > > Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 > > formats with. > > I will double-check with the office in Japan, but I don't think that > > they have one either. > > Tomi, is this something you could test ? Replying to myself, this is something we could test, but let's not delay this series, new formats can always be added on top. > > >> There's no need to send > > >> a v2, I can handle the simple change in the commit message if you let me > > >> know whether my comment is right or wrong.
Hi Laurent, On 2023/08/14 6:46, Laurent Pinchart wrote: > On Fri, Aug 04, 2023 at 06:52:51PM +0300, Laurent Pinchart wrote: >> On Fri, Aug 04, 2023 at 11:49:32AM -0400, Damian Hobson-Garcia wrote: >>> On 2023/08/03 20:06, Laurent Pinchart wrote: >>>> On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote: >>>>> On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote: >>>>>> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote: >>>>>>> Add additional pixel formats for which blending is disabling when >>>>>> Did you mean "disabled" instead of "disabling" ? >>> Oops. Yes, that's exactly what I meant. > I'll fix this locally. Thank you very much. > >>>>>>> DRM_MODE_BLEND_PIXEL_NONE is set. >>>>>>> >>>>>>> Refactor the fourcc selection into a separate function to handle the >>>>>>> increased number of formats. >>>>>>> >>>>>>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> >>>>>>> --- >>>>>>> drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- >>>>>>> 1 file changed, 32 insertions(+), 17 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c >>>>>>> index 45c05d0ffc70..96241c03b60f 100644 >>>>>>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c >>>>>>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c >>>>>>> @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { >>>>>>> DRM_FORMAT_Y212, >>>>>>> }; >>>>>>> >>>>>>> +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) >>>>>>> +{ >>>>>>> + u32 fourcc = state->format->fourcc; >>>>>>> + >>>>>>> + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { >>>>>>> + switch (fourcc) { >>>>>>> + case DRM_FORMAT_ARGB1555: >>>>>>> + fourcc = DRM_FORMAT_XRGB1555; >>>>>>> + break; >>>>>>> + >>>>>>> + case DRM_FORMAT_ARGB4444: >>>>>>> + fourcc = DRM_FORMAT_XRGB4444; >>>>>>> + break; >>>>>>> + >>>>>>> + case DRM_FORMAT_ARGB8888: >>>>>>> + fourcc = DRM_FORMAT_XRGB8888; >>>>>>> + break; >>>>>>> + >>>>>>> + case DRM_FORMAT_BGRA8888: >>>>>>> + fourcc = DRM_FORMAT_BGRX8888; >>>>>>> + break; >>>>>>> + >>>>>>> + case DRM_FORMAT_RGBA1010102: >>>>>>> + fourcc = DRM_FORMAT_RGBX1010102; >>>>>>> + break; >>>>>> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out >>>>>> intentionally ? >>> Yes, it was intentionally left out for the time being for the >>> reason that you noted (i.e. DRM_FORMAT_XRGB2101010 not >>> being handled by the DU driver). >>> >>>>> It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as >>>>> DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment. >>>>> Let's do so with a patch on top of this series. >>> Yes, I was thinking the same thing. >>> >>>> Replying to myself again, the datasheet doesn't explicitly list >>>> DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to >>>> specify the location of the components should work fine for that format. >>>> Is this something you would be able to test ? >>> Unfortunately I don't have a Gen 4 system on hand to test the 2-10-10-10 >>> formats with. >>> I will double-check with the office in Japan, but I don't think that >>> they have one either. >> Tomi, is this something you could test ? > Replying to myself, this is something we could test, but let's not delay > this series, new formats can always be added on top. Ok, great. Thanks very much. Damian >>>>> There's no need to send >>>>> a v2, I can handle the simple change in the commit message if you let me >>>>> know whether my comment is right or wrong.
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c index 45c05d0ffc70..96241c03b60f 100644 --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = { DRM_FORMAT_Y212, }; +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state) +{ + u32 fourcc = state->format->fourcc; + + if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { + switch (fourcc) { + case DRM_FORMAT_ARGB1555: + fourcc = DRM_FORMAT_XRGB1555; + break; + + case DRM_FORMAT_ARGB4444: + fourcc = DRM_FORMAT_XRGB4444; + break; + + case DRM_FORMAT_ARGB8888: + fourcc = DRM_FORMAT_XRGB8888; + break; + + case DRM_FORMAT_BGRA8888: + fourcc = DRM_FORMAT_BGRX8888; + break; + + case DRM_FORMAT_RGBA1010102: + fourcc = DRM_FORMAT_RGBX1010102; + break; + } + } + + return fourcc; +} + static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) { struct rcar_du_vsp_plane_state *state = @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) .alpha = state->state.alpha >> 8, .zpos = state->state.zpos, }; - u32 fourcc = state->format->fourcc; + u32 fourcc = rcar_du_vsp_state_get_format(state); unsigned int i; cfg.src.left = state->state.src.x1 >> 16; @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane) cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl) + fb->offsets[i]; - if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) { - switch (fourcc) { - case DRM_FORMAT_ARGB1555: - fourcc = DRM_FORMAT_XRGB1555; - break; - - case DRM_FORMAT_ARGB4444: - fourcc = DRM_FORMAT_XRGB4444; - break; - - case DRM_FORMAT_ARGB8888: - fourcc = DRM_FORMAT_XRGB8888; - break; - } - } - format = rcar_du_format_info(fourcc); cfg.pixelformat = format->v4l2;
Add additional pixel formats for which blending is disabling when DRM_MODE_BLEND_PIXEL_NONE is set. Refactor the fourcc selection into a separate function to handle the increased number of formats. Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp> --- drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-)