On 2015-04-27 08:33, Joonyoung Shim wrote:
> Hi Tobias,
>
> On 04/24/2015 05:10 PM, Tobias Jakobi wrote:
>> On 2015-04-24 03:48, Joonyoung Shim wrote:
>>> Hi Tobias,
>>>
>>> On 04/23/2015 09:12 PM, Tobias Jakobi wrote:
>>>> Move the defines for the pixelformats that the mixer supports out
>>>> of mixer_graph_buffer() to the top of the source.
>>>> Then select the mixer pixelformat (pf) in mixer_graph_buffer() based
>>>> on
>>>> the plane's pf (and not bpp).
>>>> Also add handling of RGB565 and XRGB1555 to the switch statement and
>>>> exit early if the plane has an unsupported pf.
>>>>
>>>> Partially based on 'drm/exynos: enable/disable blend based on pixel
>>>> format' by Gustavo Padovan <gustavo.padovan at collabora.co.uk>.
>>>>
>>>> Signed-off-by: Tobias Jakobi <tjakobi at math.uni-bielefeld.de>
>>>> ---
>>>> drivers/gpu/drm/exynos/exynos_mixer.c | 32
>>>> ++++++++++++++++++++++----------
>>>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> index fbec750..1bd23ee 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>> @@ -44,6 +44,12 @@
>>>> #define MIXER_WIN_NR 3
>>>> #define MIXER_DEFAULT_WIN 0
>>>>
>>>> +/* The pixelformats that are natively supported by the mixer. */
>>>> +#define MIXER_PIXELFORMAT_RGB565 4
>>>> +#define MIXER_PIXELFORMAT_ARGB1555 5
>>>> +#define MIXER_PIXELFORMAT_ARGB4444 6
>>>> +#define MIXER_PIXELFORMAT_ARGB8888 7
>>>> +
>>>
>>> Seems be long, how about s/PIXELFORMAT/FORMAT or just MIXER_RGB565
>>> ... ?
>>> Please use tab between define name and value.
>> How about MXR_FORMAT_XYZ, to stay consistent with the regs-mixer
>> header?
>>
>>
>>>> struct mixer_resources {
>>>> int irq;
>>>> void __iomem *mixer_regs;
>>>> @@ -531,20 +537,26 @@ static void mixer_graph_buffer(struct
>>>> mixer_context *ctx, int win)
>>>>
>>>> plane = &ctx->planes[win];
>>>>
>>>> - #define RGB565 4
>>>> - #define ARGB1555 5
>>>> - #define ARGB4444 6
>>>> - #define ARGB8888 7
>>>> + switch (plane->pixel_format) {
>>>> + case DRM_FORMAT_XRGB4444:
>>>> + fmt = MIXER_PIXELFORMAT_ARGB4444;
>>>> + break;
>>>> +
>>>> + case DRM_FORMAT_XRGB1555:
>>>> + fmt = MIXER_PIXELFORMAT_ARGB1555;
>>>> + break;
>>>>
>>>> - switch (plane->bpp) {
>>>> - case 16:
>>>> - fmt = ARGB4444;
>>>> + case DRM_FORMAT_RGB565:
>>>> + fmt = MIXER_PIXELFORMAT_RGB565;
>>>> break;
>>>> - case 32:
>>>> - fmt = ARGB8888;
>>>> +
>>>> + case DRM_FORMAT_XRGB8888:
>>>> + fmt = MIXER_PIXELFORMAT_ARGB8888;
>>>> break;
>>>> +
>>>> default:
>>>> - fmt = ARGB8888;
>>>> + DRM_DEBUG_KMS("pixelformat unsupported by mixer\n");
>>>> + return;
>>>> }
>>>>
>>>> /* check if mixer supports requested scaling setup */
>>>>
>>>
>>> Hmm missing formats having alpha?
>>> DRM_FORMAT_ARGB4444
>>> DRM_FORMAT_ARGB1555
>>> DRM_FORMAT_ARGB8888
>> Should be obvious from my other mail. Blending/alpha is currently
>> screwed up, so we shouldn't add these formats at this point. At least
>> not before figuring out how to properly solve the issue.
>>
>
> It shouldn't mean to remove current supported feature(alpha pixel
> format
> plane support of mixer driver).
Ah, I see what you mean! Going to send an updated version later this
day.
With best wishes,
Tobias
>
> Thanks.
