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.
> Thanks.
With best wishes,
Tobias
