[Nouveau] [PATCH 0/2] drm/nouveau: add support for 2560x1440@56 over HDMI
On 08/25/2015 07:09 AM, Ben Skeggs wrote: > On 18 August 2015 at 04:04, Hauke Mehrtens wrote: >> On 08/08/2015 07:01 PM, Hauke Mehrtens wrote: >>> These patches are adding support for outputting 2560x1440 at 56 over HDMI. >>> This needs a pixel clock of 225 MHz which was not supported before. >>> >>> This was tested in a dual monitor setup with a GF114 (GTX 560 TI) and >>> one HDMI monitor running with 2560x1440 at 56 and one DVI monitor running >>> with 1920x1200 at 60. This still needs testing on other graphics cards and >>> with dual link DVI. > Out of curiosity, what mode does the VBIOS select to display the > system's boot messages? The monitor is not activated before X comes up, I see the boot messages only on my monitor connected via DVI. You can find my vbios here: https://www.hauke-m.de/files/.nouveau/vbios.rom Hauke
[PATCH 10/14] exynos/fimg2d: remove default case from g2d_get_blend_op()
On 2015ë 08ì 24ì¼ 23:14, Tobias Jakobi wrote: > We now validate the blending mode via g2d_validate_mode() > prior to feeding it to g2d_get_blend_op(). > > Signed-off-by: Tobias Jakobi > --- > exynos/exynos_fimg2d.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c > index 4274a94..d708e91 100644 > --- a/exynos/exynos_fimg2d.c > +++ b/exynos/exynos_fimg2d.c > @@ -91,11 +91,6 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op) > SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0, > G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0); > break; > - default: > - fprintf(stderr, "Not support operation(%d).\n", op); > - SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO, > - 0, 0, 0); > - break; With this, how about changing above switch and case statement to if statement? Thanks, Inki Dae > } > > return val.val; >
[PATCH 09/14] exynos/fimg2d: check buffer space in g2d_scale_and_blend()
On 2015ë
08ì 24ì¼ 23:14, Tobias Jakobi wrote:
> Apply the same transformation as in g2d_blend().
>
> Signed-off-by: Tobias Jakobi
> ---
> exynos/exynos_fimg2d.c | 67
> +-
> 1 file changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> index 5acccf8..4274a94 100644
> --- a/exynos/exynos_fimg2d.c
> +++ b/exynos/exynos_fimg2d.c
> @@ -745,9 +745,47 @@ g2d_scale_and_blend(struct g2d_context *ctx, struct
> g2d_image *src,
> union g2d_point_val pt;
> union g2d_bitblt_cmd_val bitblt;
> union g2d_blend_func_val blend;
> - unsigned int scale;
> + unsigned int scale, gem_space;
> unsigned int scale_x, scale_y;
>
> + if (src_w == dst_w && src_h == dst_h)
> + scale = 0;
> + else {
> + scale = 1;
> + scale_x = g2d_get_scaling(src_w, dst_w);
> + scale_y = g2d_get_scaling(src_h, dst_h);
> + }
> +
> + if (src_x + src_w > src->width)
> + src_w = src->width - src_x;
> + if (src_y + src_h > src->height)
> + src_h = src->height - src_y;
> +
> + if (dst_x + dst_w > dst->width)
> + dst_w = dst->width - dst_x;
> + if (dst_y + dst_h > dst->height)
> + dst_h = dst->height - dst_y;
> +
> + if (src_w <= 0 || src_h <= 0 || dst_w <= 0 || dst_h <= 0) {
> + fprintf(stderr, "invalid width or height.\n");
> + return -EINVAL;
> + }
> +
> + if (g2d_validate_select_mode(src->select_mode)) {
> + fprintf(stderr , "invalid select mode for source.\n");
> + return -EINVAL;
> + }
> +
> + if (g2d_validate_blending_op(op)) {
> + fprintf(stderr , "unsupported blending operation.\n");
> + return -EINVAL;
> + }
> +
> + gem_space = src->select_mode == G2D_SELECT_MODE_NORMAL ? 2 : 1;
> +
> + if (g2d_check_space(ctx, 12 + scale * 3, gem_space))
> + return -ENOSPC;
Ditto.
Thanks,
Inki Dae
> +
> bitblt.val = 0;
> blend.val = 0;
>
> @@ -774,33 +812,6 @@ g2d_scale_and_blend(struct g2d_context *ctx, struct
> g2d_image *src,
> case G2D_SELECT_MODE_BGCOLOR:
> g2d_add_cmd(ctx, BG_COLOR_REG, src->color);
> break;
> - default:
> - fprintf(stderr , "failed to set src.\n");
> - return -EINVAL;
> - }
> -
> - if (src_w == dst_w && src_h == dst_h)
> - scale = 0;
> - else {
> - scale = 1;
> - scale_x = g2d_get_scaling(src_w, dst_w);
> - scale_y = g2d_get_scaling(src_h, dst_h);
> - }
> -
> - if (src_x + src_w > src->width)
> - src_w = src->width - src_x;
> - if (src_y + src_h > src->height)
> - src_h = src->height - src_y;
> -
> - if (dst_x + dst_w > dst->width)
> - dst_w = dst->width - dst_x;
> - if (dst_y + dst_h > dst->height)
> - dst_h = dst->height - dst_y;
> -
> - if (src_w <= 0 || src_h <= 0 || dst_w <= 0 || dst_h <= 0) {
> - fprintf(stderr, "invalid width or height.\n");
> - g2d_reset(ctx);
> - return -EINVAL;
> }
>
> if (scale) {
>
[PATCH 08/14] exynos/fimg2d: check buffer space in g2d_blend()
On 2015ë
08ì 24ì¼ 23:14, Tobias Jakobi wrote:
> Move parameter validation to the top and also validate
> the select mode of the source image and the requested
> blending operation before starting command submission.
>
> Signed-off-by: Tobias Jakobi
> ---
> exynos/exynos_fimg2d.c | 66
> +-
> 1 file changed, 39 insertions(+), 27 deletions(-)
>
> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> index 781aff5..5acccf8 100644
> --- a/exynos/exynos_fimg2d.c
> +++ b/exynos/exynos_fimg2d.c
> @@ -623,7 +623,45 @@ g2d_blend(struct g2d_context *ctx, struct g2d_image *src,
> union g2d_point_val pt;
> union g2d_bitblt_cmd_val bitblt;
> union g2d_blend_func_val blend;
> - unsigned int src_w = 0, src_h = 0, dst_w = 0, dst_h = 0;
> + unsigned int gem_space;
> + unsigned int src_w, src_h, dst_w, dst_h;
> +
> + src_w = w;
> + src_h = h;
> + if (src_x + w > src->width)
> + src_w = src->width - src_x;
> + if (src_y + h > src->height)
> + src_h = src->height - src_y;
> +
> + dst_w = w;
> + dst_h = h;
> + if (dst_x + w > dst->width)
> + dst_w = dst->width - dst_x;
> + if (dst_y + h > dst->height)
> + dst_h = dst->height - dst_y;
> +
> + w = MIN(src_w, dst_w);
> + h = MIN(src_h, dst_h);
> +
> + if (w <= 0 || h <= 0) {
> + fprintf(stderr, "invalid width or height.\n");
> + return -EINVAL;
> + }
> +
> + if (g2d_validate_select_mode(src->select_mode)) {
> + fprintf(stderr , "invalid select mode for source.\n");
> + return -EINVAL;
> + }
> +
> + if (g2d_validate_blending_op(op)) {
> + fprintf(stderr , "unsupported blending operation.\n");
> + return -EINVAL;
> + }
> +
> + gem_space = src->select_mode == G2D_SELECT_MODE_NORMAL ? 2 : 1;
> +
> + if (g2d_check_space(ctx, 12, gem_space))
> + return -ENOSPC;
As I mentioned before, above two lines could be integrated with other
patches.
Thanks,
Inki Dae
>
> bitblt.val = 0;
> blend.val = 0;
> @@ -651,32 +689,6 @@ g2d_blend(struct g2d_context *ctx, struct g2d_image *src,
> case G2D_SELECT_MODE_BGCOLOR:
> g2d_add_cmd(ctx, BG_COLOR_REG, src->color);
> break;
> - default:
> - fprintf(stderr , "failed to set src.\n");
> - return -EINVAL;
> - }
> -
> - src_w = w;
> - src_h = h;
> - if (src_x + w > src->width)
> - src_w = src->width - src_x;
> - if (src_y + h > src->height)
> - src_h = src->height - src_y;
> -
> - dst_w = w;
> - dst_h = h;
> - if (dst_x + w > dst->width)
> - dst_w = dst->width - dst_x;
> - if (dst_y + h > dst->height)
> - dst_h = dst->height - dst_y;
> -
> - w = MIN(src_w, dst_w);
> - h = MIN(src_h, dst_h);
> -
> - if (w <= 0 || h <= 0) {
> - fprintf(stderr, "invalid width or height.\n");
> - g2d_reset(ctx);
> - return -EINVAL;
> }
>
> bitblt.data.alpha_blend_mode = G2D_ALPHA_BLEND_MODE_ENABLE;
>
[PATCH 07/14] exynos/fimg2d: add g2d_validate_xyz() functions
On 2015ë
08ì 24ì¼ 23:14, Tobias Jakobi wrote:
> The G2D headers define a number of modes through enums
> (like e.g. color, select, repeat, etc.).
>
> This introduces g2d_validate_select_mode() and
> g2d_validate_blending_op() which validate a
> select mode or blending operation respectively.
>
> Signed-off-by: Tobias Jakobi
> ---
> exynos/exynos_fimg2d.c | 44
> 1 file changed, 44 insertions(+)
>
> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> index 2e04f4a..781aff5 100644
> --- a/exynos/exynos_fimg2d.c
> +++ b/exynos/exynos_fimg2d.c
> @@ -119,6 +119,50 @@ static unsigned int g2d_check_space(const struct
> g2d_context *ctx,
> }
>
> /*
> + * g2d_validate_select_mode - validate select mode.
> + *
> + * @mode: the mode to validate
> + */
> +static unsigned int g2d_validate_select_mode(
> + enum e_g2d_select_mode mode)
> +{
> + switch (mode) {
> + case G2D_SELECT_MODE_NORMAL:
> + case G2D_SELECT_MODE_FGCOLOR:
> + case G2D_SELECT_MODE_BGCOLOR:
> + return 0;
> + }
It's strange use a bit. Just check the range like below,
First, please add G2D_SELECT_MODE_MAX which has 3 << 0, and
if (G2D_SELECT_MODE_MAX < mode) {
fprintf(strerr, "invalid command(0x%x)\n", mode);
return -EINVAL;
}
And I think it'd be better to change return type of this function to int,
> +
> + return 1;
so return 0
> +}
> +
> +/*
> + * g2d_validate_blending_op - validate blending operation.
> + *
> + * @operation: the operation to validate
> + */
> +static unsigned int g2d_validate_blending_op(
> + enum e_g2d_op operation)
> +{
> + switch (operation) {
> + case G2D_OP_CLEAR:
> + case G2D_OP_SRC:
> + case G2D_OP_DST:
> + case G2D_OP_OVER:
> + case G2D_OP_INTERPOLATE:
> + case G2D_OP_DISJOINT_CLEAR:
> + case G2D_OP_DISJOINT_SRC:
> + case G2D_OP_DISJOINT_DST:
> + case G2D_OP_CONJOINT_CLEAR:
> + case G2D_OP_CONJOINT_SRC:
> + case G2D_OP_CONJOINT_DST:
> + return 0;
Ditto, You could modify it like above.
Thanks,
Inki Dae
> + }
> +
> + return 1;
> +}
> +
> +/*
> * g2d_add_cmd - set given command and value to user side command buffer.
> *
> * @ctx: a pointer to g2d_context structure.
>
[PATCH 05/14] exynos/fimg2d: check buffer space in g2d_copy()
On 2015ë
08ì 24ì¼ 23:14, Tobias Jakobi wrote:
> Move the parameter validation before buffer space checking
> so that we can exit early if it fails.
> Also don't reset the G2D context anymore in this situation
> (since the buffers are not partially submitted).
>
> Signed-off-by: Tobias Jakobi
> ---
> exynos/exynos_fimg2d.c | 26 ++
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> index 9b7bcce..185aa80 100644
> --- a/exynos/exynos_fimg2d.c
> +++ b/exynos/exynos_fimg2d.c
> @@ -375,17 +375,7 @@ g2d_copy(struct g2d_context *ctx, struct g2d_image *src,
> {
> union g2d_rop4_val rop4;
> union g2d_point_val pt;
> - unsigned int src_w = 0, src_h = 0, dst_w = 0, dst_h = 0;
> -
> - g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_BGCOLOR);
> - g2d_add_cmd(ctx, DST_COLOR_MODE_REG, dst->color_mode);
> - g2d_add_base_addr(ctx, dst, g2d_dst);
> - g2d_add_cmd(ctx, DST_STRIDE_REG, dst->stride);
> -
> - g2d_add_cmd(ctx, SRC_SELECT_REG, G2D_SELECT_MODE_NORMAL);
> - g2d_add_cmd(ctx, SRC_COLOR_MODE_REG, src->color_mode);
> - g2d_add_base_addr(ctx, src, g2d_src);
> - g2d_add_cmd(ctx, SRC_STRIDE_REG, src->stride);
> + unsigned int src_w, src_h, dst_w, dst_h;
>
> src_w = w;
> src_h = h;
> @@ -406,10 +396,22 @@ g2d_copy(struct g2d_context *ctx, struct g2d_image *src,
>
> if (w <= 0 || h <= 0) {
> fprintf(stderr, "invalid width or height.\n");
> - g2d_reset(ctx);
> return -EINVAL;
> }
>
> + if (g2d_check_space(ctx, 11, 2))
> + return -ENOSPC;
Above lines could be integrated with 3 and 4 patches as one patch. And
you can make other codes to other one.
Thanks,
Inki Dae
> +
> + g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_BGCOLOR);
> + g2d_add_cmd(ctx, DST_COLOR_MODE_REG, dst->color_mode);
> + g2d_add_base_addr(ctx, dst, g2d_dst);
> + g2d_add_cmd(ctx, DST_STRIDE_REG, dst->stride);
> +
> + g2d_add_cmd(ctx, SRC_SELECT_REG, G2D_SELECT_MODE_NORMAL);
> + g2d_add_cmd(ctx, SRC_COLOR_MODE_REG, src->color_mode);
> + g2d_add_base_addr(ctx, src, g2d_src);
> + g2d_add_cmd(ctx, SRC_STRIDE_REG, src->stride);
> +
> pt.val = 0;
> pt.data.x = src_x;
> pt.data.y = src_y;
>
[PATCH 04/14] exynos/fimg2d: check buffer space in g2d_solid_fill()
On 2015ë 08ì 24ì¼ 23:13, Tobias Jakobi wrote: > The amount of commands (regular and GEM) doesn't depend > on the input here. > > Signed-off-by: Tobias Jakobi > --- > exynos/exynos_fimg2d.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c > index 1ae8adf..9b7bcce 100644 > --- a/exynos/exynos_fimg2d.c > +++ b/exynos/exynos_fimg2d.c > @@ -319,6 +319,9 @@ g2d_solid_fill(struct g2d_context *ctx, struct g2d_image > *img, > union g2d_bitblt_cmd_val bitblt; > union g2d_point_val pt; > > + if (g2d_check_space(ctx, 7, 1)) > + return -ENOSPC; You can make 3 and 4 patches to one. These should be same patch. > + > g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_NORMAL); > g2d_add_cmd(ctx, DST_COLOR_MODE_REG, img->color_mode); > g2d_add_base_addr(ctx, img, g2d_dst); >
drm/exynos: add render node support
Hi Emil, On 2015ë 08ì 31ì¼ 20:58, Emil Velikov wrote: > Hi all, > > On 18 August 2015 at 08:01, Inki Dae wrote: >> From: Joonyoung Shim >> >> This patch allows clients who want to use render node to access >> rendering relevant ioctls - g2d, post processor and gem allocation. >> > I seem to recall Dave and others saying that we must have users of > these interfaces prior to merging them into the kernel. Then again I > cannot find any open-source userspace using the post-processor. Can > anyone kingly share a link ? You can refer to the user-space below, https://review.tizen.org/git/?p=platform/upstream/libdrm.git;a=commit;h=52de1c57e2e6f3b2a1259478d5ae260ed4c5706e > > On the g2d front, can you guys take a look at the work Tobias has been > doing ? He seems to be the only one that cares about it :'( I'd like to say really sorry about that to you and Tobias. Will review it soon. Thanks, Inki Dae > > Thanks > Emil >
drm/exynos: add render node support
Emil Velikov wrote: > On 31 August 2015 at 13:36, Inki Dae wrote: >> Hi Emil, >> >> On 2015ë 08ì 31ì¼ 20:58, Emil Velikov wrote: >>> Hi all, >>> >>> On 18 August 2015 at 08:01, Inki Dae wrote: From: Joonyoung Shim This patch allows clients who want to use render node to access rendering relevant ioctls - g2d, post processor and gem allocation. >>> I seem to recall Dave and others saying that we must have users of >>> these interfaces prior to merging them into the kernel. Then again I >>> cannot find any open-source userspace using the post-processor. Can >>> anyone kingly share a link ? >> >> You can refer to the user-space below, >> https://review.tizen.org/git/?p=platform/upstream/libdrm.git;a=commit;h=52de1c57e2e6f3b2a1259478d5ae260ed4c5706e >> > Nice one thanks ! > Did those patch(es) ever made it to the list ? Mind giving them a tiny > bit of cleanup (move tests to exynos, drop duplication, C99 > initializers) and sending them over ? > > On a mildly related note: > The tizen people like excessive rebasing :( Another alternative would > be to send fixes upstream and keep their changes within ./packaging, > which will allow clean merges. > >>> >>> On the g2d front, can you guys take a look at the work Tobias has been >>> doing ? He seems to be the only one that cares about it :'( >> >> I'd like to say really sorry about that to you and Tobias. Will review >> it soon. >> > Great. Tobias sent a few more drm/exynos and libdrm (g2d utilities) > patches that've been around for a while :( Thanks, the size two patches for the kernel (size check) are kinda important. Currently userspace can easily trigger a pagefault due to this being wrong. So maybe it's also a candidate for stable (but I don't know too much about how stable backports work). With best wishes, Tobias P.S.: I also might've some patches ready in 2 weeks or so which should resolve the g2d/userptr issues. > > Perhaps he can re-spin then and/or send a list with outstanding ones ? > > Cheers, > Emil >
[PATCH 07/14] exynos/fimg2d: add g2d_validate_xyz() functions
Hello,
Emil Velikov wrote:
> On 31 August 2015 at 14:18, Inki Dae wrote:
>> On 2015ë
08ì 24ì¼ 23:14, Tobias Jakobi wrote:
>>> The G2D headers define a number of modes through enums
>>> (like e.g. color, select, repeat, etc.).
>>>
>>> This introduces g2d_validate_select_mode() and
>>> g2d_validate_blending_op() which validate a
>>> select mode or blending operation respectively.
>>>
>>> Signed-off-by: Tobias Jakobi
>>> ---
>>> exynos/exynos_fimg2d.c | 44
>>> 1 file changed, 44 insertions(+)
>>>
>>> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
>>> index 2e04f4a..781aff5 100644
>>> --- a/exynos/exynos_fimg2d.c
>>> +++ b/exynos/exynos_fimg2d.c
>>> @@ -119,6 +119,50 @@ static unsigned int g2d_check_space(const struct
>>> g2d_context *ctx,
>>> }
>>>
>>> /*
>>> + * g2d_validate_select_mode - validate select mode.
>>> + *
>>> + * @mode: the mode to validate
>>> + */
>>> +static unsigned int g2d_validate_select_mode(
>>> + enum e_g2d_select_mode mode)
>>> +{
>>> + switch (mode) {
>>> + case G2D_SELECT_MODE_NORMAL:
>>> + case G2D_SELECT_MODE_FGCOLOR:
>>> + case G2D_SELECT_MODE_BGCOLOR:
>>> + return 0;
>>> + }
>>
>> It's strange use a bit. Just check the range like below,
>>
>> First, please add G2D_SELECT_MODE_MAX which has 3 << 0, and
>>
>> if (G2D_SELECT_MODE_MAX < mode) {
>> fprintf(strerr, "invalid command(0x%x)\n", mode);
>> return -EINVAL;
>> }
>>
> Listing every value might seem like an overkill, indeed. Yet I think
> it's the more robust approach.
that's my thinking as well. The overhead shouldn't be too high and the
compiler probably optimizes this anyway.
> By adding _MAX to the API we effectively lock it down. That can be
> avoided, plus the compiler will warn us when new values are added to
> the enum. If someone starts getting smart (due to the _MAX) and adds
> G2D_SELECT_MODE_FOO = -1, the above check will fail :(
>
>> And I think it'd be better to change return type of this function to int,
>>
> Good idea.
What would be the benefit of this? We're just returning only 0 and 1
anyway. My first reaction was to use a bool here.
>
> Cheers,
> Emil
>
With best wishes,
Tobias
[PATCH 04/14] exynos/fimg2d: check buffer space in g2d_solid_fill()
Hello! Inki Dae wrote: > On 2015ë 08ì 24ì¼ 23:13, Tobias Jakobi wrote: >> The amount of commands (regular and GEM) doesn't depend >> on the input here. >> >> Signed-off-by: Tobias Jakobi >> --- >> exynos/exynos_fimg2d.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c >> index 1ae8adf..9b7bcce 100644 >> --- a/exynos/exynos_fimg2d.c >> +++ b/exynos/exynos_fimg2d.c >> @@ -319,6 +319,9 @@ g2d_solid_fill(struct g2d_context *ctx, struct g2d_image >> *img, >> union g2d_bitblt_cmd_val bitblt; >> union g2d_point_val pt; >> >> +if (g2d_check_space(ctx, 7, 1)) >> +return -ENOSPC; > > You can make 3 and 4 patches to one. These should be same patch. Hmm, so which 3 (respectively 4) patches should be squashed? I tried to separate stuff to make review easier. If squashing here is the only issue, do I need to resend the series or can e.g. Emil just do the squash when merging? With best wishes, Tobias > >> + >> g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_NORMAL); >> g2d_add_cmd(ctx, DST_COLOR_MODE_REG, img->color_mode); >> g2d_add_base_addr(ctx, img, g2d_dst); >> >
[PATCH 04/14] exynos/fimg2d: check buffer space in g2d_solid_fill()
On 31 August 2015 at 20:27, Tobias Jakobi wrote: > Hello! > > Inki Dae wrote: >> On 2015ë 08ì 24ì¼ 23:13, Tobias Jakobi wrote: >>> +if (g2d_check_space(ctx, 7, 1)) >>> +return -ENOSPC; >> >> You can make 3 and 4 patches to one. These should be same patch. > Hmm, so which 3 (respectively 4) patches should be squashed? > I believe he meant "squash the introduction of the function and its uses into a single patch". Not sure how much value that brings, so I'll let you guys decide on the bike shed colour :-) -Emil
[PATCH libdrm 4.1/17] intel: introduce to_bo_gem() helper
On Mon, Aug 31, 2015 at 08:47:17PM +0100, Emil Velikov wrote: > ...to minimise misuse of bo_gem. > If the variable is declared at the top of the function and then used > for two (or more) different contexts this can cause confusion and errors. > > Just introduce a wrapper, which can be used in a once off situations. > > Suggested-by: Chris Wilson > Cc: Chris Wilson > Signed-off-by: Emil Velikov Reviewed-by Chris Wilson (And may as well take that as r-b for the preceding patch then) -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH libdrm 4.1/17] intel: introduce to_bo_gem() helper
...to minimise misuse of bo_gem.
If the variable is declared at the top of the function and then used
for two (or more) different contexts this can cause confusion and errors.
Just introduce a wrapper, which can be used in a once off situations.
Suggested-by: Chris Wilson
Cc: Chris Wilson
Signed-off-by: Emil Velikov
---
intel/intel_bufmgr_gem.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 5287419..63122d0 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -276,6 +276,11 @@ static void drm_intel_gem_bo_unreference(drm_intel_bo *bo);
static void drm_intel_gem_bo_free(drm_intel_bo *bo);
+static inline drm_intel_bo_gem *to_bo_gem(drm_intel_bo *bo)
+{
+return (drm_intel_bo_gem *)bo;
+}
+
static unsigned long
drm_intel_gem_bo_tile_size(drm_intel_bufmgr_gem *bufmgr_gem, unsigned long
size,
uint32_t *tiling_mode)
@@ -2116,11 +2121,10 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used,
drm_clip_rect_t * cliprects, int num_cliprects, int DR4)
{
drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
- drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
struct drm_i915_gem_execbuffer execbuf;
int ret, i;
- if (bo_gem->has_error)
+ if (to_bo_gem(bo)->has_error)
return -ENOMEM;
pthread_mutex_lock(&bufmgr_gem->lock);
@@ -2165,7 +2169,7 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used,
drm_intel_gem_dump_validation_list(bufmgr_gem);
for (i = 0; i < bufmgr_gem->exec_count; i++) {
- bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
+ drm_intel_bo_gem *bo_gem = to_bo_gem(bufmgr_gem->exec_bos[i]);
bo_gem->idle = false;
@@ -2185,12 +2189,11 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context
*ctx,
unsigned int flags)
{
drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
- drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
struct drm_i915_gem_execbuffer2 execbuf;
int ret = 0;
int i;
- if (bo_gem->has_error)
+ if (to_bo_gem(bo)->has_error)
return -ENOMEM;
switch (flags & 0x7) {
@@ -2263,7 +2266,7 @@ skip_execution:
drm_intel_gem_dump_validation_list(bufmgr_gem);
for (i = 0; i < bufmgr_gem->exec_count; i++) {
- bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
+ drm_intel_bo_gem *bo_gem = to_bo_gem(bufmgr_gem->exec_bos[i]);
bo_gem->idle = false;
--
2.5.0
[Bug 91828] R600 LLVM Assertion in Instructions.cpp:1499
https://bugs.freedesktop.org/show_bug.cgi?id=91828 Bug ID: 91828 Summary: R600 LLVM Assertion in Instructions.cpp:1499 Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/r600 Assignee: dri-devel at lists.freedesktop.org Reporter: ikalvachev at gmail.com QA Contact: dri-devel at lists.freedesktop.org Created attachment 118020 --> https://bugs.freedesktop.org/attachment.cgi?id=118020&action=edit Log of running Xonotic with R600_DEBUG=llvm,ps,vs. The last shader causes assertion. This bug happen with wide variety of games. I managed to reproduce it with the free Xonotic v0.8.1. The exact command is `R600_DEBUG=llvm,ps,vs ./xonotic-linux32-glx`. (Then start the first single player map. Crash happens at first in-game frame.) The error I get is: --- xonotic-linux32-glx: Instructions.cpp:1499: llvm::InsertElementInst::InsertElementInst(llvm::Value *, llvm::Value *, llvm::Value *, const llvm::Twine &, llvm::Instruction *): Assertion `isValidOperands(Vec, Elt, Index) && "Invalid insertelement instruction operands!"' failed. --- I've obtained that error by compiling debug versions of LLVM 3.6.2 and current git of Mesa. Unfortunately the backtrace doesn't produce any relevant info. I'm reporting the bug here, because the error happens during the phase where TGSI code is translated into the intrinsics that LLVM understand. I do not see llvm intrinsics code for the last TGSI shader, so I cannot reproduce the crash with `llc`. I think it is quite likely that the problem is caused by some LLVM API change, that have not been reflected in Mesa. I know that R600 LLVM is not under active development. However at the moment that code is crashing on regular basis. Mesa should be able to work with any recent LLVM release and 3.6.2 is the latest current release. If you do not intend to maintain it, then you might consider removing it. I would prefer the bug(s) to be fixed. Nine exposes quite a number of SB bugs and having other options is always good for testing. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150831/ed03a6e8/attachment.html>
[PATCH libdrm v2] intel: error out on has_error in exec2
On Mon, Aug 31, 2015 at 08:01:21PM +0100, Emil Velikov wrote:
> > Reusing bo_gem here is a little worrying as it would be very easy for
> > someone to add code to the end of the function thinking that bo_gem
> > still was the batch.
> >
> Doesn't this concert apply to drm_intel_gem_bo_exec() as well ?
Probably. Haven't looked at that in years. :p
> > If we had
> >
> > static inline drm_intel_bo_gem *to_bo_gem(drm_intel_bo *bo)
> > {
> > return (drm_intel_bo_gem *)bo;
> > }
> >
> > then we can start doing one offs like
> >
> > if (to_bo_gem(bo)->has_error) return -ENOMEM;
> >
> > and of course
> > for (i = 0; i < bufmgr_gem->exec_count; i++) {
> > drm_intel_bo_gem *bo_gem =
> > to_bo_gem(bufmgr_gem->exec_bos[i]);
>
> How about we do this as a follow up patch (4.1/17) that covers both functions
> ?
If you are quick...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH libdrm v2] intel: error out on has_error in exec2
On 31 August 2015 at 19:26, Chris Wilson wrote:
> On Mon, Aug 31, 2015 at 07:14:12PM +0100, Emil Velikov wrote:
>> Just like we do for the original exec()
>>
>> v2: move bo_gem declaration to the top of the function.
>>
>> Cc: Chris Wilson
>> Cc: intel-gfx at lists.freedesktop.org
>> Signed-off-by: Emil Velikov
>> ---
>> intel/intel_bufmgr_gem.c | 7 +--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
>> index 7303903..5287419 100644
>> --- a/intel/intel_bufmgr_gem.c
>> +++ b/intel/intel_bufmgr_gem.c
>> @@ -2185,10 +2185,14 @@ do_exec2(drm_intel_bo *bo, int used,
>> drm_intel_context *ctx,
>>unsigned int flags)
>> {
>> drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
>> + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
>> struct drm_i915_gem_execbuffer2 execbuf;
>> int ret = 0;
>> int i;
>>
>> + if (bo_gem->has_error)
>> + return -ENOMEM;
>> +
>> switch (flags & 0x7) {
>> default:
>> return -EINVAL;
>> @@ -2259,8 +2263,7 @@ skip_execution:
>> drm_intel_gem_dump_validation_list(bufmgr_gem);
>>
>> for (i = 0; i < bufmgr_gem->exec_count; i++) {
>> - drm_intel_bo_gem *bo_gem =
>> - (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
>> + bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
>
> Reusing bo_gem here is a little worrying as it would be very easy for
> someone to add code to the end of the function thinking that bo_gem
> still was the batch.
>
Doesn't this concert apply to drm_intel_gem_bo_exec() as well ?
> If we had
>
> static inline drm_intel_bo_gem *to_bo_gem(drm_intel_bo *bo)
> {
> return (drm_intel_bo_gem *)bo;
> }
>
> then we can start doing one offs like
>
> if (to_bo_gem(bo)->has_error) return -ENOMEM;
>
> and of course
> for (i = 0; i < bufmgr_gem->exec_count; i++) {
> drm_intel_bo_gem *bo_gem = to_bo_gem(bufmgr_gem->exec_bos[i]);
How about we do this as a follow up patch (4.1/17) that covers both functions ?
Thanks again,
Emil
[PATCH 10/14] exynos/fimg2d: remove default case from g2d_get_blend_op()
On 31 August 2015 at 14:25, Inki Dae wrote: > On 2015ë 08ì 24ì¼ 23:14, Tobias Jakobi wrote: >> We now validate the blending mode via g2d_validate_mode() >> prior to feeding it to g2d_get_blend_op(). >> >> Signed-off-by: Tobias Jakobi >> --- >> exynos/exynos_fimg2d.c | 5 - >> 1 file changed, 5 deletions(-) >> >> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c >> index 4274a94..d708e91 100644 >> --- a/exynos/exynos_fimg2d.c >> +++ b/exynos/exynos_fimg2d.c >> @@ -91,11 +91,6 @@ static unsigned int g2d_get_blend_op(enum e_g2d_op op) >> SET_BF(val, G2D_COEFF_MODE_SRC_ALPHA, 0, 0, 0, >> G2D_COEFF_MODE_SRC_ALPHA, 1, 0, 0); >> break; >> - default: >> - fprintf(stderr, "Not support operation(%d).\n", op); >> - SET_BF(val, G2D_COEFF_MODE_ONE, 0, 0, 0, G2D_COEFF_MODE_ZERO, >> - 0, 0, 0); >> - break; > > With this, how about changing above switch and case statement to if > statement? > Out of curiosity: why is if/else statement preferred - won't it make things longer/harder to read (there are 11 cases here) ? Cheers, Emil
[PATCH 07/14] exynos/fimg2d: add g2d_validate_xyz() functions
On 31 August 2015 at 14:18, Inki Dae wrote:
> On 2015ë
08ì 24ì¼ 23:14, Tobias Jakobi wrote:
>> The G2D headers define a number of modes through enums
>> (like e.g. color, select, repeat, etc.).
>>
>> This introduces g2d_validate_select_mode() and
>> g2d_validate_blending_op() which validate a
>> select mode or blending operation respectively.
>>
>> Signed-off-by: Tobias Jakobi
>> ---
>> exynos/exynos_fimg2d.c | 44
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
>> index 2e04f4a..781aff5 100644
>> --- a/exynos/exynos_fimg2d.c
>> +++ b/exynos/exynos_fimg2d.c
>> @@ -119,6 +119,50 @@ static unsigned int g2d_check_space(const struct
>> g2d_context *ctx,
>> }
>>
>> /*
>> + * g2d_validate_select_mode - validate select mode.
>> + *
>> + * @mode: the mode to validate
>> + */
>> +static unsigned int g2d_validate_select_mode(
>> + enum e_g2d_select_mode mode)
>> +{
>> + switch (mode) {
>> + case G2D_SELECT_MODE_NORMAL:
>> + case G2D_SELECT_MODE_FGCOLOR:
>> + case G2D_SELECT_MODE_BGCOLOR:
>> + return 0;
>> + }
>
> It's strange use a bit. Just check the range like below,
>
> First, please add G2D_SELECT_MODE_MAX which has 3 << 0, and
>
> if (G2D_SELECT_MODE_MAX < mode) {
> fprintf(strerr, "invalid command(0x%x)\n", mode);
> return -EINVAL;
> }
>
Listing every value might seem like an overkill, indeed. Yet I think
it's the more robust approach.
By adding _MAX to the API we effectively lock it down. That can be
avoided, plus the compiler will warn us when new values are added to
the enum. If someone starts getting smart (due to the _MAX) and adds
G2D_SELECT_MODE_FOO = -1, the above check will fail :(
> And I think it'd be better to change return type of this function to int,
>
Good idea.
Cheers,
Emil
drm/exynos: add render node support
On 31 August 2015 at 13:36, Inki Dae wrote: > Hi Emil, > > On 2015ë 08ì 31ì¼ 20:58, Emil Velikov wrote: >> Hi all, >> >> On 18 August 2015 at 08:01, Inki Dae wrote: >>> From: Joonyoung Shim >>> >>> This patch allows clients who want to use render node to access >>> rendering relevant ioctls - g2d, post processor and gem allocation. >>> >> I seem to recall Dave and others saying that we must have users of >> these interfaces prior to merging them into the kernel. Then again I >> cannot find any open-source userspace using the post-processor. Can >> anyone kingly share a link ? > > You can refer to the user-space below, > https://review.tizen.org/git/?p=platform/upstream/libdrm.git;a=commit;h=52de1c57e2e6f3b2a1259478d5ae260ed4c5706e > Nice one thanks ! Did those patch(es) ever made it to the list ? Mind giving them a tiny bit of cleanup (move tests to exynos, drop duplication, C99 initializers) and sending them over ? On a mildly related note: The tizen people like excessive rebasing :( Another alternative would be to send fixes upstream and keep their changes within ./packaging, which will allow clean merges. >> >> On the g2d front, can you guys take a look at the work Tobias has been >> doing ? He seems to be the only one that cares about it :'( > > I'd like to say really sorry about that to you and Tobias. Will review > it soon. > Great. Tobias sent a few more drm/exynos and libdrm (g2d utilities) patches that've been around for a while :( Perhaps he can re-spin then and/or send a list with outstanding ones ? Cheers, Emil
[PATCH libdrm v2] intel: error out on has_error in exec2
On Mon, Aug 31, 2015 at 07:14:12PM +0100, Emil Velikov wrote:
> Just like we do for the original exec()
>
> v2: move bo_gem declaration to the top of the function.
>
> Cc: Chris Wilson
> Cc: intel-gfx at lists.freedesktop.org
> Signed-off-by: Emil Velikov
> ---
> intel/intel_bufmgr_gem.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index 7303903..5287419 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -2185,10 +2185,14 @@ do_exec2(drm_intel_bo *bo, int used,
> drm_intel_context *ctx,
>unsigned int flags)
> {
> drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
> + drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
> struct drm_i915_gem_execbuffer2 execbuf;
> int ret = 0;
> int i;
>
> + if (bo_gem->has_error)
> + return -ENOMEM;
> +
> switch (flags & 0x7) {
> default:
> return -EINVAL;
> @@ -2259,8 +2263,7 @@ skip_execution:
> drm_intel_gem_dump_validation_list(bufmgr_gem);
>
> for (i = 0; i < bufmgr_gem->exec_count; i++) {
> - drm_intel_bo_gem *bo_gem =
> - (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
> + bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
Reusing bo_gem here is a little worrying as it would be very easy for
someone to add code to the end of the function thinking that bo_gem
still was the batch.
If we had
static inline drm_intel_bo_gem *to_bo_gem(drm_intel_bo *bo)
{
return (drm_intel_bo_gem *)bo;
}
then we can start doing one offs like
if (to_bo_gem(bo)->has_error) return -ENOMEM;
and of course
for (i = 0; i < bufmgr_gem->exec_count; i++) {
drm_intel_bo_gem *bo_gem = to_bo_gem(bufmgr_gem->exec_bos[i]);
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH libdrm v2] intel: resolve shadowing warnings
On Mon, Aug 31, 2015 at 07:13:12PM +0100, Emil Velikov wrote: > v2: keep the bo_gem declaration in exec2() within the loop (Chris) > > Cc: Chris Wilson > Cc: intel-gfx at lists.freedesktop.org > Signed-off-by: Emil Velikov Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH libdrm v2] intel: error out on has_error in exec2
Just like we do for the original exec()
v2: move bo_gem declaration to the top of the function.
Cc: Chris Wilson
Cc: intel-gfx at lists.freedesktop.org
Signed-off-by: Emil Velikov
---
intel/intel_bufmgr_gem.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 7303903..5287419 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -2185,10 +2185,14 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context
*ctx,
unsigned int flags)
{
drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bo->bufmgr;
+ drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
struct drm_i915_gem_execbuffer2 execbuf;
int ret = 0;
int i;
+ if (bo_gem->has_error)
+ return -ENOMEM;
+
switch (flags & 0x7) {
default:
return -EINVAL;
@@ -2259,8 +2263,7 @@ skip_execution:
drm_intel_gem_dump_validation_list(bufmgr_gem);
for (i = 0; i < bufmgr_gem->exec_count; i++) {
- drm_intel_bo_gem *bo_gem =
- (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
+ bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
bo_gem->idle = false;
--
2.5.0
[PATCH libdrm v2] intel: resolve shadowing warnings
v2: keep the bo_gem declaration in exec2() within the loop (Chris)
Cc: Chris Wilson
Cc: intel-gfx at lists.freedesktop.org
Signed-off-by: Emil Velikov
---
intel/intel_bufmgr_fake.c | 2 +-
intel/intel_bufmgr_gem.c | 7 +++
intel/intel_decode.c | 7 ++-
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/intel/intel_bufmgr_fake.c b/intel/intel_bufmgr_fake.c
index 75387b7..551e05d 100644
--- a/intel/intel_bufmgr_fake.c
+++ b/intel/intel_bufmgr_fake.c
@@ -1460,7 +1460,7 @@ restart:
assert(ret == 0);
if (bufmgr_fake->exec != NULL) {
- int ret = bufmgr_fake->exec(bo, used, bufmgr_fake->exec_priv);
+ ret = bufmgr_fake->exec(bo, used, bufmgr_fake->exec_priv);
if (ret != 0) {
pthread_mutex_unlock(&bufmgr_fake->lock);
return ret;
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 2723e21..7303903 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -2165,8 +2165,7 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used,
drm_intel_gem_dump_validation_list(bufmgr_gem);
for (i = 0; i < bufmgr_gem->exec_count; i++) {
- drm_intel_bo *bo = bufmgr_gem->exec_bos[i];
- drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
+ bo_gem = (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
bo_gem->idle = false;
@@ -2260,8 +2259,8 @@ skip_execution:
drm_intel_gem_dump_validation_list(bufmgr_gem);
for (i = 0; i < bufmgr_gem->exec_count; i++) {
- drm_intel_bo *bo = bufmgr_gem->exec_bos[i];
- drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *)bo;
+ drm_intel_bo_gem *bo_gem =
+ (drm_intel_bo_gem *) bufmgr_gem->exec_bos[i];
bo_gem->idle = false;
diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 2b902a3..345d457 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -3630,7 +3630,6 @@ decode_3d_965(struct drm_intel_decode *ctx)
case 0x7a00:
if (IS_GEN6(devid) || IS_GEN7(devid)) {
- unsigned int i;
if (len != 4 && len != 5)
fprintf(out, "Bad count in PIPE_CONTROL\n");
@@ -3732,8 +3731,6 @@ decode_3d_965(struct drm_intel_decode *ctx)
if (opcode_3d->func) {
return opcode_3d->func(ctx);
} else {
- unsigned int i;
-
instr_out(ctx, 0, "%s\n", opcode_3d->name);
for (i = 1; i < len; i++) {
@@ -3883,9 +3880,9 @@ drm_intel_decode_set_head_tail(struct drm_intel_decode
*ctx,
void
drm_intel_decode_set_output_file(struct drm_intel_decode *ctx,
-FILE *out)
+FILE *output)
{
- ctx->out = out;
+ ctx->out = output;
}
/**
--
2.5.0
[PATCH 1/2] drm/radeon/native: Send out the full AUX address
On 31.08.2015 17:19, Alex Deucher wrote: > AUX addresses are 20 bits long. Send out the entire address instead of > just the low 16 bits. > > Port of: > drm/radeon/atom: Send out the full AUX address > to radeon non-atom aux path > > Signed-off-by: Alex Deucher > Cc: stable at vger.kernel.org Reviewed-by: Christian König for both patches. > --- > drivers/gpu/drm/radeon/radeon_dp_auxch.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_dp_auxch.c > b/drivers/gpu/drm/radeon/radeon_dp_auxch.c > index fcbd60b..3b0c229 100644 > --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c > +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c > @@ -116,8 +116,8 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, > struct drm_dp_aux_msg *msg > AUX_SW_WR_BYTES(bytes)); > > /* write the data header into the registers */ > - /* request, addres, msg size */ > - byte = (msg->request << 4); > + /* request, address, msg size */ > + byte = (msg->request << 4) | ((msg->address >> 16) & 0xf); > WREG32(AUX_SW_DATA + aux_offset[instance], > AUX_SW_DATA_MASK(byte) | AUX_SW_AUTOINCREMENT_DISABLE); >
[PATCH] drm/fsl-dcu: Add multi layers support
For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610),
add (total_layer - 1) overlay planes.
Signed-off-by: Jianwei Wang
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 8787920..1195568 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -236,7 +236,10 @@ static const u32 fsl_dcu_drm_plane_formats[] = {
struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
{
+ struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
struct drm_plane *primary;
+ struct drm_plane *overlay;
+ unsigned int total_layer, formats_size, i;
int ret;
primary = kzalloc(sizeof(*primary), GFP_KERNEL);
@@ -245,11 +248,12 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct
drm_device *dev)
return NULL;
}
+ formats_size = ARRAY_SIZE(fsl_dcu_drm_plane_formats);
/* possible_crtc's will be filled in later by crtc_init */
ret = drm_universal_plane_init(dev, primary, 0,
&fsl_dcu_drm_plane_funcs,
fsl_dcu_drm_plane_formats,
- ARRAY_SIZE(fsl_dcu_drm_plane_formats),
+ formats_size,
DRM_PLANE_TYPE_PRIMARY);
if (ret) {
kfree(primary);
@@ -257,5 +261,26 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct
drm_device *dev)
}
drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
+ total_layer = fsl_dev->soc->total_layer;
+ for (i = 0; i < total_layer - 1; i++) {
+ overlay = kzalloc(sizeof(*overlay), GFP_KERNEL);
+ if (!overlay) {
+ DRM_DEBUG_KMS("Failed to allocate overlay plane\n");
+ goto out;
+ }
+
+ ret = drm_universal_plane_init(dev, overlay, 1,
+ &fsl_dcu_drm_plane_funcs,
+ fsl_dcu_drm_plane_formats,
+ formats_size,
+ DRM_PLANE_TYPE_OVERLAY);
+ if (ret) {
+ kfree(overlay);
+ goto out;
+ }
+ drm_plane_helper_add(overlay, &fsl_dcu_drm_plane_helper_funcs);
+ }
+
+out:
return primary;
}
--
2.1.0.27.g96db324
[PATCH v3 RESEND 0/5] Remaining part of kill off set_irq_flags usage
On Sat, 29 Aug 2015, Rob Herring wrote: > Thomas, > > As requested, here are the remaining patches for killing off > set_irq_flags which have not been picked up. The rest of the > series has been picked up and are in -next. I pick it up, stick it into tip irq/urgent let it brew for a couple of days in next and send it to Linus after that. Thanks, tglx
[PATCH 4/4] drm: Constify loads of function pointer structs
From: Ville Syrjälä Moves a bunch of junk to .rodata from .data. drivers/gpu/drm/armada/armada.ko: -.rodata 1040 +.rodata 1100 -.data1156 +.data1096 drivers/gpu/drm/atmel-hlcdc/atmel-hlcdc-dc.ko: -.text 12488 +.text 12480 -.rodata 1696 +.rodata 1760 -.data 776 +.data 712 drivers/gpu/drm/bochs/bochs-drm.ko: -.text7608 +.text7600 -.rodata 648 +.rodata 716 -.data 612 +.data 544 drivers/gpu/drm/bridge/dw_hdmi.ko: -.rodata 120 +.rodata 216 -.data 96 +.data 0 drivers/gpu/drm/bridge/nxp-ptn3460.ko: -.rodata 440 +.rodata 536 -.data208 +.data112 drivers/gpu/drm/cirrus/cirrus.ko: -.text 10104 +.text 10092 -.rodata 528 +.rodata 596 -.data 608 +.data 540 drivers/gpu/drm/exynos/exynosdrm.ko: -.text 125792 +.text 125788 -.rodata 10972 +.rodata 11748 -.data 6720 +.data 5944 drivers/gpu/drm/i2c/adv7511.ko: -.rodata 1368 +.rodata 1416 -.data164 +.data116 drivers/gpu/drm/i2c/ch7006.ko: -.text5752 +.text5760 -.rodata 6608 +.rodata 6656 -.data 216 +.data 168 drivers/gpu/drm/i2c/sil164.ko: -.text 1660 +.text 1656 -.rodata 56 +.rodata 104 -.data212 +.data164 drivers/gpu/drm/i2c/tda998x.ko: -.rodata 668 +.rodata 716 -.data212 +.data164 drivers/gpu/drm/imx/dw_hdmi-imx.ko: -.rodata 768 +.rodata 824 -.data188 +.data132 drivers/gpu/drm/imx/imxdrm.ko: -.rodata 624 +.rodata 652 -.data372 +.data344 drivers/gpu/drm/imx/imx-ipuv3-crtc.ko: -.rodata 224 +.rodata 280 -.data184 +.data128 drivers/gpu/drm/imx/imx-ldb.ko: -.rodata 660 +.rodata 784 -.data240 +.data116 drivers/gpu/drm/imx/imx-tve.ko: -.rodata 400 +.rodata 524 -.data416 +.data292 drivers/gpu/drm/imx/parallel-display.ko: -.rodata 400 +.rodata 524 -.data216 +.data 92 drivers/gpu/drm/mgag200/mgag200.ko: -.text 29244 +.text 29232 -.rodata 600 +.rodata 668 -.data 688 +.data 620 drivers/gpu/drm/nouveau/nouveau.ko: -.rodata 105688 +.rodata 105792 -.data125724 +.data125620 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.ko: -.rodata 772 +.rodata 828 -.data148 +.data 92 drivers/gpu/drm/rockchip/rockchipdrm.ko: -.rodata 748 +.rodata 760 -.data448 +.data436 drivers/gpu/drm/sti/sticompositor.ko: -.text 12216 +.text 12212 -.rodata 1284 +.rodata 1400 -.data 488 +.data 372 drivers/gpu/drm/sti/sti_drv.ko: -.rodata 516 +.rodata 544 -.data368 +.data340 drivers/gpu/drm/sti/stidvo.ko: -.text 3356 +.text 3348 -.rodata 188 +.rodata 256 -.data572 +.data504 drivers/gpu/drm/sti/sti_hda.ko: -.text 3008 +.text
[PATCH 3/4] drm/i2c/ch7006: Constify ch7006_tv_norms[] and ch7006_modes[]
From: Ville Syrjälä
drivers/gpu/drm/i2c/ch7006.ko:
-.text5913
+.text5897
-.rodata 664
+.rodata 7256
-.data6992
+.data 416
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i2c/ch7006_mode.c | 22 +++---
drivers/gpu/drm/i2c/ch7006_priv.h | 10 +-
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i2c/ch7006_mode.c
b/drivers/gpu/drm/i2c/ch7006_mode.c
index f4dca53..bb5f67f 100644
--- a/drivers/gpu/drm/i2c/ch7006_mode.c
+++ b/drivers/gpu/drm/i2c/ch7006_mode.c
@@ -46,7 +46,7 @@ const char * const ch7006_tv_norm_names[] = {
.vtotal = 625, \
.hvirtual = 810
-struct ch7006_tv_norm_info ch7006_tv_norms[] = {
+const struct ch7006_tv_norm_info ch7006_tv_norms[] = {
[TV_NORM_NTSC_M] = {
NTSC_LIKE_TIMINGS,
.black_level = 0.339 * fixed1,
@@ -142,7 +142,7 @@ struct ch7006_tv_norm_info ch7006_tv_norms[] = {
#define PAL_LIKE (1 << TV_NORM_PAL | 1 << TV_NORM_PAL_N | 1 << TV_NORM_PAL_NC)
-struct ch7006_mode ch7006_modes[] = {
+const struct ch7006_mode ch7006_modes[] = {
MODE(21000, 512, 384, 840, 500, N, N, 181.797557582, 5_4, 0x6,
PAL_LIKE),
MODE(26250, 512, 384, 840, 625, N, N, 145.438046066, 1_1, 0x1,
PAL_LIKE),
MODE(20140, 512, 384, 800, 420, N, N, 213.257083791, 5_4, 0x4,
NTSC_LIKE),
@@ -171,11 +171,11 @@ struct ch7006_mode ch7006_modes[] = {
{}
};
-struct ch7006_mode *ch7006_lookup_mode(struct drm_encoder *encoder,
- const struct drm_display_mode *drm_mode)
+const struct ch7006_mode *ch7006_lookup_mode(struct drm_encoder *encoder,
+const struct drm_display_mode
*drm_mode)
{
struct ch7006_priv *priv = to_ch7006_priv(encoder);
- struct ch7006_mode *mode;
+ const struct ch7006_mode *mode;
for (mode = ch7006_modes; mode->mode.clock; mode++) {
@@ -233,8 +233,8 @@ void ch7006_setup_subcarrier(struct drm_encoder *encoder)
struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
struct ch7006_priv *priv = to_ch7006_priv(encoder);
struct ch7006_state *state = &priv->state;
- struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
- struct ch7006_mode *mode = priv->mode;
+ const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
+ const struct ch7006_mode *mode = priv->mode;
uint32_t subc_inc;
subc_inc = round_fixed((mode->subc_coeff >> 8)
@@ -257,7 +257,7 @@ void ch7006_setup_pll(struct drm_encoder *encoder)
struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
struct ch7006_priv *priv = to_ch7006_priv(encoder);
uint8_t *regs = priv->state.regs;
- struct ch7006_mode *mode = priv->mode;
+ const struct ch7006_mode *mode = priv->mode;
int n, best_n = 0;
int m, best_m = 0;
int freq, best_freq = 0;
@@ -328,9 +328,9 @@ void ch7006_setup_properties(struct drm_encoder *encoder)
struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
struct ch7006_priv *priv = to_ch7006_priv(encoder);
struct ch7006_state *state = &priv->state;
- struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
- struct ch7006_mode *ch_mode = priv->mode;
- struct drm_display_mode *mode = &ch_mode->mode;
+ const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
+ const struct ch7006_mode *ch_mode = priv->mode;
+ const struct drm_display_mode *mode = &ch_mode->mode;
uint8_t *regs = state->regs;
int flicker, contrast, hpos, vpos;
uint64_t scale, aspect;
diff --git a/drivers/gpu/drm/i2c/ch7006_priv.h
b/drivers/gpu/drm/i2c/ch7006_priv.h
index cef6ce7..dc6414a 100644
--- a/drivers/gpu/drm/i2c/ch7006_priv.h
+++ b/drivers/gpu/drm/i2c/ch7006_priv.h
@@ -78,7 +78,7 @@ struct ch7006_state {
struct ch7006_priv {
struct ch7006_encoder_params params;
- struct ch7006_mode *mode;
+ const struct ch7006_mode *mode;
struct ch7006_state state;
struct ch7006_state saved_state;
@@ -107,11 +107,11 @@ extern char *ch7006_tv_norm;
extern int ch7006_scale;
extern const char * const ch7006_tv_norm_names[];
-extern struct ch7006_tv_norm_info ch7006_tv_norms[];
-extern struct ch7006_mode ch7006_modes[];
+extern const struct ch7006_tv_norm_info ch7006_tv_norms[];
+extern const struct ch7006_mode ch7006_modes[];
-struct ch7006_mode *ch7006_lookup_mode(struct drm_encoder *encoder,
- const struct drm_display_mode *drm_mode);
+const struct ch7006_mode *ch7006_lookup_mode(struct drm_encoder *encoder,
+const struct drm_display_mode
*drm_mode);
[PATCH 2/4] drm: Constify TV mode names
From: Ville Syrjälä
Make the mode names passed to drm_mode_create_tv_properties() const.
drivers/gpu/drm/i2c/ch7006.ko:
-.rodata 596
+.rodata 664
-.data7064
+.data6992
drivers/gpu/drm/nouveau/nouveau.ko:
-.rodata 146808
+.rodata 146904
-.data178624
+.data178528
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_crtc.c | 2 +-
drivers/gpu/drm/i2c/ch7006_drv.c | 6 +++---
drivers/gpu/drm/i2c/ch7006_mode.c | 4 ++--
drivers/gpu/drm/i2c/ch7006_priv.h | 2 +-
drivers/gpu/drm/i915/intel_tv.c| 4 ++--
drivers/gpu/drm/nouveau/dispnv04/tvmodesnv17.c | 2 +-
drivers/gpu/drm/nouveau/dispnv04/tvnv17.h | 2 +-
include/drm/drm_crtc.h | 2 +-
8 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 884690c..474f328 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1519,7 +1519,7 @@ EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
*/
int drm_mode_create_tv_properties(struct drm_device *dev,
unsigned int num_modes,
- char *modes[])
+ const char * const modes[])
{
struct drm_property *tv_selector;
struct drm_property *tv_subconnector;
diff --git a/drivers/gpu/drm/i2c/ch7006_drv.c b/drivers/gpu/drm/i2c/ch7006_drv.c
index 51fa323..d9a72c9 100644
--- a/drivers/gpu/drm/i2c/ch7006_drv.c
+++ b/drivers/gpu/drm/i2c/ch7006_drv.c
@@ -119,8 +119,8 @@ static void ch7006_encoder_mode_set(struct drm_encoder
*encoder,
struct ch7006_encoder_params *params = &priv->params;
struct ch7006_state *state = &priv->state;
uint8_t *regs = state->regs;
- struct ch7006_mode *mode = priv->mode;
- struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
+ const struct ch7006_mode *mode = priv->mode;
+ const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
int start_active;
ch7006_dbg(client, "\n");
@@ -226,7 +226,7 @@ static int ch7006_encoder_get_modes(struct drm_encoder
*encoder,
struct drm_connector *connector)
{
struct ch7006_priv *priv = to_ch7006_priv(encoder);
- struct ch7006_mode *mode;
+ const struct ch7006_mode *mode;
int n = 0;
for (mode = ch7006_modes; mode->mode.clock; mode++) {
diff --git a/drivers/gpu/drm/i2c/ch7006_mode.c
b/drivers/gpu/drm/i2c/ch7006_mode.c
index 9b83574..f4dca53 100644
--- a/drivers/gpu/drm/i2c/ch7006_mode.c
+++ b/drivers/gpu/drm/i2c/ch7006_mode.c
@@ -26,7 +26,7 @@
#include "ch7006_priv.h"
-char *ch7006_tv_norm_names[] = {
+const char * const ch7006_tv_norm_names[] = {
[TV_NORM_PAL] = "PAL",
[TV_NORM_PAL_M] = "PAL-M",
[TV_NORM_PAL_N] = "PAL-N",
@@ -202,7 +202,7 @@ void ch7006_setup_levels(struct drm_encoder *encoder)
struct i2c_client *client = drm_i2c_encoder_get_client(encoder);
struct ch7006_priv *priv = to_ch7006_priv(encoder);
uint8_t *regs = priv->state.regs;
- struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
+ const struct ch7006_tv_norm_info *norm = &ch7006_tv_norms[priv->norm];
int gain;
int black_level;
diff --git a/drivers/gpu/drm/i2c/ch7006_priv.h
b/drivers/gpu/drm/i2c/ch7006_priv.h
index ce57784..cef6ce7 100644
--- a/drivers/gpu/drm/i2c/ch7006_priv.h
+++ b/drivers/gpu/drm/i2c/ch7006_priv.h
@@ -106,7 +106,7 @@ extern int ch7006_debug;
extern char *ch7006_tv_norm;
extern int ch7006_scale;
-extern char *ch7006_tv_norm_names[];
+extern const char * const ch7006_tv_norm_names[];
extern struct ch7006_tv_norm_info ch7006_tv_norms[];
extern struct ch7006_mode ch7006_modes[];
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index cbe39dc..29983cba 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1579,7 +1579,7 @@ intel_tv_init(struct drm_device *dev)
struct intel_encoder *intel_encoder;
struct intel_connector *intel_connector;
u32 tv_dac_on, tv_dac_off, save_tv_dac;
- char *tv_format_names[ARRAY_SIZE(tv_modes)];
+ const char *tv_format_names[ARRAY_SIZE(tv_modes)];
int i, initial_mode = 0;
if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
@@ -1677,7 +1677,7 @@ intel_tv_init(struct drm_device *dev)
/* Create TV properties then attach current values */
for (i = 0; i < ARRAY_SIZE(tv_modes); i++)
- tv_format_names[i] = (char *)tv_modes[i].name;
+ tv_format_names[i] = tv_modes[i].name;
drm_mode_create_tv_properties(dev,
[PATCH 1/4] drm: Constify generic_edid_names[]
From: Ville Syrjälä
Make generic_edid_names[] const since it's supposed to be immutable.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid_load.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 1f445e9..698b8c3 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -32,7 +32,7 @@ MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use
specified EDID blob "
"from built-in data or /lib/firmware instead. ");
#define GENERIC_EDIDS 6
-static const char *generic_edid_name[GENERIC_EDIDS] = {
+static const char * const generic_edid_name[GENERIC_EDIDS] = {
"edid/800x600.bin",
"edid/1024x768.bin",
"edid/1280x1024.bin",
--
2.4.6
[Intel-gfx] [PATCH v2 12/22] drm/i915: Preserve SSC earlier
On 07/15/2015 04:57 AM, Lukas Wunner wrote:
> Commit 92122789b2d6 ("drm/i915: preserve SSC if previously set v3")
> added code to intel_modeset_gem_init to override the SSC status read
> from VBT with the SSC status set by BIOS.
>
> However, intel_modeset_gem_init is invoked *after* intel_modeset_init,
> which calls intel_setup_outputs, which *modifies* SSC status by way of
> intel_init_pch_refclk. So unlike advertised, intel_modeset_gem_init
> doesn't preserve the SSC status set by BIOS but whatever
> intel_init_pch_refclk decided on.
>
> This is a problem on dual gpu laptops such as the MacBook Pro which
> require either a handler to switch DDC lines, or the discrete gpu
> to proxy DDC/AUX communication: Both the handler and the discrete
> gpu may initialize after the i915 driver, and consequently, an LVDS
> connector may initially seem disconnected and the SSC therefore
> is disabled by intel_init_pch_refclk, but on reprobe the connector
> may turn out to be connected and the SSC must then be enabled.
>
> Due to 92122789b2d6 however, the SSC is not enabled on reprobe since
> it is assumed BIOS disabled it while in fact it was disabled by
> intel_init_pch_refclk.
>
> Also, because the SSC status is preserved so late, the preserved value
> only ever gets used on resume but not on panel initialization:
> intel_modeset_init calls intel_init_display which indirectly calls
> intel_panel_use_ssc via multiple subroutines, *before* the BIOS value
> overrides the VBT value in intel_modeset_gem_init (intel_panel_use_ssc
> is the sole user of dev_priv->vbt.lvds_use_ssc).
>
> Fix this by moving the code introduced by 92122789b2d6 from
> intel_modeset_gem_init to intel_modeset_init before the invocation
> of intel_setup_outputs and intel_init_display.
>
> Add a DRM_DEBUG_KMS as suggested way back by Jani:
> http://lists.freedesktop.org/archives/intel-gfx/2014-June/04.html
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
> Tested-by: Paul Hordiienko
> [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina]
> Tested-by: William Brown
> [MBP 8,2 2011 intel SNB + amd turks pre-retina]
> Tested-by: Lukas Wunner
> [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina]
> Tested-by: Bruno Bierbaumer
> [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]
>
> Fixes: 92122789b2d6 ("drm/i915: preserve SSC if previously set v3")
> Signed-off-by: Lukas Wunner
> ---
> drivers/gpu/drm/i915/intel_display.c | 29 ++---
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index af0bcfe..6335883 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14893,6 +14893,24 @@ void intel_modeset_init(struct drm_device *dev)
> if (INTEL_INFO(dev)->num_pipes == 0)
> return;
>
> + /*
> + * There may be no VBT; and if the BIOS enabled SSC we can
> + * just keep using it to avoid unnecessary flicker. Whereas if the
> + * BIOS isn't using it, don't assume it will work even if the VBT
> + * indicates as much.
> + */
> + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
> + bool bios_lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) &
> + DREF_SSC1_ENABLE);
> +
> + if (dev_priv->vbt.lvds_use_ssc != bios_lvds_use_ssc) {
> + DRM_DEBUG_KMS("SSC %sabled by BIOS, overriding VBT
> which says %sabled\n",
> + bios_lvds_use_ssc ? "en" : "dis",
> + dev_priv->vbt.lvds_use_ssc ? "en" : "dis");
> + dev_priv->vbt.lvds_use_ssc = bios_lvds_use_ssc;
> + }
> + }
> +
> intel_init_display(dev);
> intel_init_audio(dev);
>
> @@ -15446,7 +15464,6 @@ err:
>
> void intel_modeset_gem_init(struct drm_device *dev)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *c;
> struct drm_i915_gem_object *obj;
> int ret;
> @@ -15455,16 +15472,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
> intel_init_gt_powersave(dev);
> mutex_unlock(&dev->struct_mutex);
>
> - /*
> - * There may be no VBT; and if the BIOS enabled SSC we can
> - * just keep using it to avoid unnecessary flicker. Whereas if the
> - * BIOS isn't using it, don't assume it will work even if the VBT
> - * indicates as much.
> - */
> - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
> - dev_priv->vbt.lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) &
> - DREF_SSC1_ENABLE);
> -
> intel_modeset_init_hw(dev);
>
> intel_setup_overlay(dev);
>
Yeah looks good (and I'm having deja vu here; I thought I ran into the same
o
[PATCH] drm/fsl-dcu: Cleanup vblank interrupt mask and status setting code
Switch update interrupt mask bit with regmap_update_bits, and clear
interrupt status by writing 1 to relevant bit before setting mask in
fsl_dcu_drm_irq_init function.
Signed-off-by: Jianwei Wang
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 26 +++---
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 9a8e2da..bf36971 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -38,21 +38,17 @@ static const struct regmap_config fsl_dcu_regmap_config = {
static int fsl_dcu_drm_irq_init(struct drm_device *dev)
{
struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- unsigned int value;
int ret;
ret = drm_irq_install(dev, fsl_dev->irq);
if (ret < 0)
dev_err(dev->dev, "failed to install IRQ handler\n");
- ret = regmap_write(fsl_dev->regmap, DCU_INT_STATUS, 0);
+ ret = regmap_write(fsl_dev->regmap, DCU_INT_STATUS, 0x);
if (ret)
dev_err(dev->dev, "set DCU_INT_STATUS failed\n");
- ret = regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
- if (ret)
- dev_err(dev->dev, "read DCU_INT_MASK failed\n");
- value &= DCU_INT_MASK_VBLANK;
- ret = regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
+ ret = regmap_update_bits(fsl_dev->regmap, DCU_INT_MASK,
+DCU_INT_MASK_VBLANK, ~DCU_INT_MASK_VBLANK);
if (ret)
dev_err(dev->dev, "set DCU_INT_MASK failed\n");
ret = regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
@@ -143,14 +139,10 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
static int fsl_dcu_drm_enable_vblank(struct drm_device *dev, int crtc)
{
struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- unsigned int value;
int ret;
- ret = regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
- if (ret)
- dev_err(dev->dev, "read DCU_INT_MASK failed\n");
- value &= ~DCU_INT_MASK_VBLANK;
- ret = regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
+ ret = regmap_update_bits(fsl_dev->regmap, DCU_INT_MASK,
+DCU_INT_MASK_VBLANK, ~DCU_INT_MASK_VBLANK);
if (ret)
dev_err(dev->dev, "set DCU_INT_MASK failed\n");
return 0;
@@ -159,14 +151,10 @@ static int fsl_dcu_drm_enable_vblank(struct drm_device
*dev, int crtc)
static void fsl_dcu_drm_disable_vblank(struct drm_device *dev, int crtc)
{
struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
- unsigned int value;
int ret;
- ret = regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
- if (ret)
- dev_err(dev->dev, "read DCU_INT_MASK failed\n");
- value |= DCU_INT_MASK_VBLANK;
- ret = regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
+ ret = regmap_update_bits(fsl_dev->regmap, DCU_INT_MASK,
+DCU_INT_MASK_VBLANK, DCU_INT_MASK_VBLANK);
if (ret)
dev_err(dev->dev, "set DCU_INT_MASK failed\n");
}
--
2.1.0.27.g96db324
[Bug 91808] trine1 misrender r600g
https://bugs.freedesktop.org/show_bug.cgi?id=91808 --- Comment #4 from Alex Deucher --- (In reply to Martin Bednar from comment #3) > From what I gather, 66067 is about colors & luminosity. I reported this > because of the line that goes vertically through the whole screen. On the > screenshot it is located under "paused". > The sources of the line are AFAICS the gears just above the spiky area. > Specifying now just to make sure that we're talking about the same thing. I'm not sure what line you are talking about. Can you post a shot of what the rendering should look like? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150831/a84eff7c/attachment-0001.html>
drm/exynos: add render node support
Hi all, On 18 August 2015 at 08:01, Inki Dae wrote: > From: Joonyoung Shim > > This patch allows clients who want to use render node to access > rendering relevant ioctls - g2d, post processor and gem allocation. > I seem to recall Dave and others saying that we must have users of these interfaces prior to merging them into the kernel. Then again I cannot find any open-source userspace using the post-processor. Can anyone kingly share a link ? On the g2d front, can you guys take a look at the work Tobias has been doing ? He seems to be the only one that cares about it :'( Thanks Emil
[PATCH] drm/atomic: Fix bookkeeping with TEST_ONLY, v3.
Commit ec9f932ed41622d120de52a5b525e4d77b9ef17e
"drm/atomic: Cleanup on error properly in the atomic ioctl."
cleaned up some error paths, but didn't fix the TEST_ONLY path.
In the check only case plane->fb shouldn't be updated, and
the vblank events should be cleared as on failure.
Changes since v1:
- Fix -EDEADLK handling of vblank events too.
- Free state last with CHECK_ONLY.
Changes since v2:
- Add comment about freeing crtc_state->event with TEST_ONLY.
(Daniel Stone)
Signed-off-by: Maarten Lankhorst
Reviewed-by: Daniel Stone
---
drivers/gpu/drm/drm_atomic.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e515261d0a7f..940f80bbf192 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1540,7 +1540,8 @@ retry:
copied_props++;
}
- if (obj->type == DRM_MODE_OBJECT_PLANE && count_props) {
+ if (obj->type == DRM_MODE_OBJECT_PLANE && count_props &&
+ !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) {
plane = obj_to_plane(obj);
plane_mask |= (1 << drm_plane_index(plane));
plane->old_fb = plane->fb;
@@ -1562,10 +1563,11 @@ retry:
}
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
+ /*
+* Unlike commit, check_only does not clean up state.
+* Below we call drm_atomic_state_free for it.
+*/
ret = drm_atomic_check_only(state);
- /* _check_only() does not free state, unlike _commit() */
- if (!ret)
- drm_atomic_state_free(state);
} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
ret = drm_atomic_async_commit(state);
} else {
@@ -1592,25 +1594,30 @@ out:
plane->old_fb = NULL;
}
+ if (ret && arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
+ /*
+* TEST_ONLY and PAGE_FLIP_EVENT are mutually exclusive,
+* if they weren't, this code should be called on success
+* for TEST_ONLY too.
+*/
+
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ if (!crtc_state->event)
+ continue;
+
+ destroy_vblank_event(dev, file_priv,
+crtc_state->event);
+ }
+ }
+
if (ret == -EDEADLK) {
drm_atomic_state_clear(state);
drm_modeset_backoff(&ctx);
goto retry;
}
- if (ret) {
- if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!crtc_state->event)
- continue;
-
- destroy_vblank_event(dev, file_priv,
-crtc_state->event);
- }
- }
-
+ if (ret || arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
drm_atomic_state_free(state);
- }
drm_modeset_drop_locks(&ctx);
drm_modeset_acquire_fini(&ctx);
--
2.1.0
[PATCH 0/6] Properly detect swiotlb.
On Wed, Aug 26, 2015 at 2:52 PM, wrote: > So this is only build tested as i am away from hardware right now. > Idea is to provide reliable way to check if swiotlb is in use for > a device or not. It seems swiotlb_nr_tbl() is no longer reliable > for that. > > Please test. Even with these patches applied, we are still ending up in the ttm_dma_populate() path in the amdgpu driver rather than ttm_pool_populate() even with a hw iommu enabled. Any idea? The attached patch gives us a 20% performance boost in some apps, but obviously has potential issues if a hw iommu is not present. Alex > > Cheers, > Jérôme Glisse > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- next part -- A non-text attachment was scrubbed... Name: 0001-drm-amdgpu-use-ttm_pool-instead-of-ttm_dma.patch Type: text/x-patch Size: 1320 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150831/e403de42/attachment.bin>
[PATCH] drm/fsl-dcu: Fix no fb check bug
For state->fb may be NULL in fsl_dcu_drm_plane_atomic_check function,
if so, return -EINVAL. No need check in fsl_dcu_drm_plane_atomic_update
anymore.
Signed-off-by: Jianwei Wang
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 82be6b8..8787920 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -41,6 +41,9 @@ static int fsl_dcu_drm_plane_atomic_check(struct drm_plane
*plane,
{
struct drm_framebuffer *fb = state->fb;
+ if (!fb)
+ return -EINVAL;
+
switch (fb->pixel_format) {
case DRM_FORMAT_RGB565:
case DRM_FORMAT_RGB888:
@@ -84,9 +87,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane
*plane,
unsigned int alpha, bpp;
int index, ret;
- if (!fb)
- return;
-
index = fsl_dcu_drm_plane_index(plane);
if (index < 0)
return;
--
2.1.0.27.g96db324
[PATCH v3 2/2] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support
Hi Vladimir, Am Montag, den 31.08.2015, 00:34 +0300 schrieb Vladimir Zapolskiy: > The change adds support of internal HDMI I2C master controller, this > subdevice is used by default, if "ddc-i2c-bus" DT property is omitted. I think this should be mentioned in Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt > The main purpose of this functionality is to support reading EDID from > an HDMI monitor on boards, which don't have an I2C bus connected to > DDC pins. > > The current implementation does not support "I2C Master Interface > Extended Read Mode" to read data addressed by non-zero segment > pointer, this means that if EDID has more than 1 extension blocks, > EDID reading operation won't succeed, in my practice all tested HDMI > monitors have at maximum one extension block. > > Signed-off-by: Vladimir Zapolskiy > --- > The change is based on today's v4.2, please let me know, if it should be > rebased. > > The change has compilation dependency on I2CM_ADDRESS register name fix, > see http://lists.freedesktop.org/archives/dri-devel/2015-May/082980.html > > From http://lists.freedesktop.org/archives/dri-devel/2015-May/082981.html > v1 of the change was > > Tested-by: Philipp Zabel > > but I hesitate to add the tag here due to multiple updates in v2. Tested again, v2 still works fine on Nitrogen6X with HDMI monitor. regards Philipp
[PATCH 2/2] drm/amdgpu/atom: Send out the full AUX address
AUX addresses are 20 bits long. Send out the entire address instead of
just the low 16 bits.
Port of:
drm/radeon/atom: Send out the full AUX address
to amdgpu
Signed-off-by: Alex Deucher
Cc: stable at vger.kernel.org
---
drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
index 9ba0a7d..92b6aca 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
@@ -139,7 +139,8 @@ amdgpu_atombios_dp_aux_transfer(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *m
tx_buf[0] = msg->address & 0xff;
tx_buf[1] = msg->address >> 8;
- tx_buf[2] = msg->request << 4;
+ tx_buf[2] = (msg->request << 4) |
+ ((msg->address >> 16) & 0xf);
tx_buf[3] = msg->size ? (msg->size - 1) : 0;
switch (msg->request & ~DP_AUX_I2C_MOT) {
--
1.8.3.1
[PATCH 1/2] drm/radeon/native: Send out the full AUX address
AUX addresses are 20 bits long. Send out the entire address instead of just the low 16 bits. Port of: drm/radeon/atom: Send out the full AUX address to radeon non-atom aux path Signed-off-by: Alex Deucher Cc: stable at vger.kernel.org --- drivers/gpu/drm/radeon/radeon_dp_auxch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_dp_auxch.c b/drivers/gpu/drm/radeon/radeon_dp_auxch.c index fcbd60b..3b0c229 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c @@ -116,8 +116,8 @@ radeon_dp_aux_transfer_native(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg AUX_SW_WR_BYTES(bytes)); /* write the data header into the registers */ - /* request, addres, msg size */ - byte = (msg->request << 4); + /* request, address, msg size */ + byte = (msg->request << 4) | ((msg->address >> 16) & 0xf); WREG32(AUX_SW_DATA + aux_offset[instance], AUX_SW_DATA_MASK(byte) | AUX_SW_AUTOINCREMENT_DISABLE); -- 1.8.3.1
[PATCH 6/6] drm/radeon: Send out the full AUX address
On Thu, Aug 27, 2015 at 10:23 AM, wrote:
> From: Ville Syrjälä
>
> AUX addresses are 20 bits long. Send out the entire address instead of
> just the low 16 bits.
>
> Cc: Alex Deucher
> Cc: "Christian König"
> Signed-off-by: Ville Syrjälä
Applied. thanks!
Alex
> ---
> drivers/gpu/drm/radeon/atombios_dp.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index ca372c5..bd73b40 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -171,8 +171,9 @@ radeon_dp_aux_transfer_atom(struct drm_dp_aux *aux,
> struct drm_dp_aux_msg *msg)
> return -E2BIG;
>
> tx_buf[0] = msg->address & 0xff;
> - tx_buf[1] = msg->address >> 8;
> - tx_buf[2] = msg->request << 4;
> + tx_buf[1] = (msg->address >> 8) & 0xff;
> + tx_buf[2] = (msg->request << 4) |
> + ((msg->address >> 16) & 0xf);
> tx_buf[3] = msg->size ? (msg->size - 1) : 0;
>
> switch (msg->request & ~DP_AUX_I2C_MOT) {
> --
> 2.4.6
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm: Constify generic_edid_names[]
On Mon, Aug 31, 2015 at 8:09 AM, wrote:
> From: Ville Syrjälä
>
> Make generic_edid_names[] const since it's supposed to be immutable.
>
> Signed-off-by: Ville Syrjälä
For the series:
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/drm_edid_load.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 1f445e9..698b8c3 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -32,7 +32,7 @@ MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use
> specified EDID blob "
> "from built-in data or /lib/firmware instead. ");
>
> #define GENERIC_EDIDS 6
> -static const char *generic_edid_name[GENERIC_EDIDS] = {
> +static const char * const generic_edid_name[GENERIC_EDIDS] = {
> "edid/800x600.bin",
> "edid/1024x768.bin",
> "edid/1280x1024.bin",
> --
> 2.4.6
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/mdp5: enable clocks in hw_init and set_irqmask
On 08/28/2015 11:48 PM, Rob Clark wrote:
> On Fri, Aug 28, 2015 at 3:56 AM, Archit Taneja
> wrote:
>>
>>
>> On 08/27/2015 10:36 AM, Archit Taneja wrote:
>>>
>>>
>>>
>>> On 08/26/2015 08:42 PM, hali at codeaurora.org wrote:
>
> 2015-08-26 9:55 GMT-04:00 :
>>
>> Hi Archit,
>>
>>> mdp5_hw_init and mdp5_set_irqmask configure registers but may not have
>>> clocks enabled.
>>>
>>> Add mdp5_enable/disable calls in these funcs to ensure clocks are
>>> enabled. We need this until we get proper runtime pm support.
>>>
>>> Signed-off-by: Archit Taneja
>>> ---
>>>drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 10 --
>>>drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 2 ++
>>>2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
>>> index b1f73be..9fabfca 100644
>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
>>> @@ -24,9 +24,15 @@
>>>void mdp5_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask,
>>> uint32_t old_irqmask)
>>>{
>>> - mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_MDP_INTR_CLEAR(0),
>>> + struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms);
>>> +
>>> + mdp5_enable(mdp5_kms);
>>> +
>>> + mdp5_write(mdp5_kms, REG_MDP5_MDP_INTR_CLEAR(0),
>>> irqmask ^ (irqmask & old_irqmask));
>>> - mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_MDP_INTR_EN(0),
>>> irqmask);
>>> + mdp5_write(mdp5_kms, REG_MDP5_MDP_INTR_EN(0), irqmask);
>>> +
>>> + mdp5_disable(mdp5_kms);
>>>}
>>
>>
>> mdp5_set_irqmask() can be invoked in atomic context, clk_prepare() is
>> not
>> allowed in this function because it may cause process to sleep. We can
>> enable the clocks in the caller at initialization.
>>>
>>>
>>> Oh, oops. I missed that.
>>>
>
> iirc, it will be called with at least one spinlock held..
>
> We do already move the enable/disable_vblank() paths off to a worker
> so that we can ensure things are enabled before we get into
> update_irq().. the only other path to update_irq() should be when
> driver code does mdp_irq_register/unregister().. so maybe we should
> just require that the mdp4/mdp5 kms code only calls those when clk's
> are already enabled (which should be mostly true already, I think)
>
> BR,
> -R
Yes, the only case that not been covered is mdp5_irq_postinstall(). We
can
enable clocks in this function. Actually, this is what we are doing in
downstream test.
>>>
>>>
>>> It works fine if I put it in postinstall. I'll update the patch and
>>> resend.
>>
>>
>> So, I hit an issue in both the approaches.
>>
>> When I try modeset, I get a watchdog reset once the app closes down.
I meant modetest here*
>>
>> Looking at debug logs, it looks like the issue happens when the ioctl
>> RMFB and drm_release race with each other.
>
> hmm, this seems a bit strange.. since to do the RMFB ioctl the device
> must still be open.. do we end up w/ the RMFB being an async commit
> somehow? (Although in case of flip w/ gpu rendering still pending,
> somewhere we probably want to block on previous async commit?)
It isn't a race condition as I thought before. The RMFB ioctl isn't
asynchronous either.
The problem has to do with how msm_atomic_commit behaves when the
state we want to commit involves disabling the crtc:
In this case, we grab clocks once (in mdp5_prepare_commit), but
disable them twice (first, via mdp5_crtc_disable() in disable_outputs(),
and then in mdp5_complete_commit())
Whatever comes next will crash if it requires clocks. In this case,
drm_release's call to mdp5_crtc_cancel_pending_flip() is the first
thing that needs clocks, and it crashes there.
>
>> Within the the msm driver, this maps to mdp5_complete_commit
>> (drm_mode_rmfb path) being called before mdp5_crtc_cancel_pending_flip
>> (drm_release path). mdp5_complete_commit disables clocks, and the other
>> patch calls complete_flip, which requires clocks.
>>
>> If I wrap around complete_flip with mdp5_enable/disable calls, things
>> work fine. Although, that's not an ideal fix.
>
> I guess it is a reasonable thing to do.. but on that topic, it would
> be nice if someone had some time to look and the pending atomic
> suspend/resume/runtime-pm stuff. I haven't really had time to follow
> that, but I guess it is a good time to revisit the mdpN_enable/disable
> stuff?
Yeah, that'd be more ideal. We're looking at runtime pm adaptation now.
Thanks,
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v3 0/14] Add Analogix Core Display Port Driver
Hi Romain, å¨ 08/30/2015 08:16 PM, Romain Perier åé: > Hi, > > Could you rebase your serie onto linux-next or 4.2-rc8 ? it does not > apply here... Thanks for try to applied, and feel sorry for that failed. This v3 series was rebased on github.com/torvalds/linux.git, so I should rebase on kernel/next/linux-next.git when I'm preparing v4 series. Thanks, - Yakir > Regards, > Romain > > 2015-08-21 15:16 GMT+02:00 Thierry Reding : >> On Fri, Aug 21, 2015 at 08:24:16PM +0900, Jingoo Han wrote: >>> On 2015. 8. 21., at PM 7:01, Yakir Yang wrote: Hi Jingoo, > å¨ 2015/8/21 16:20, Jingoo Han åé: >> On 2015. 8. 19., at PM 11:48, Yakir Yang wrote: > . > >> .../bindings/video/analogix_dp-rockchip.txt| 83 ++ >> .../devicetree/bindings/video/exynos_dp.txt| 51 +- >> arch/arm/boot/dts/exynos5250-arndale.dts | 10 +- >> arch/arm/boot/dts/exynos5250-smdk5250.dts | 10 +- >> arch/arm/boot/dts/exynos5250-snow.dts | 12 +- >> arch/arm/boot/dts/exynos5250-spring.dts| 12 +- >> arch/arm/boot/dts/exynos5420-peach-pit.dts | 12 +- >> arch/arm/boot/dts/exynos5420-smdk5420.dts | 10 +- >> arch/arm/boot/dts/exynos5800-peach-pi.dts | 12 +- >> drivers/gpu/drm/bridge/Kconfig |5 + >> drivers/gpu/drm/bridge/Makefile|1 + >> drivers/gpu/drm/bridge/analogix_dp_core.c | 1382 >> +++ >> drivers/gpu/drm/bridge/analogix_dp_core.h | 286 >> drivers/gpu/drm/bridge/analogix_dp_reg.c | 1294 >> ++ >> .../exynos_dp_reg.h => bridge/analogix_dp_reg.h} | 270 ++-- >> drivers/gpu/drm/exynos/Kconfig |5 +- >> drivers/gpu/drm/exynos/Makefile|2 +- >> drivers/gpu/drm/exynos/analogix_dp-exynos.c| 347 + > Would you change this file name to "exynos_dp.c"? Sorry, I don't think so ;( I think IP_name+Soc_name would be better in this re-use case. >>> So? Is there the naming rule such as "IP_name+SoC_name"? >>> Beside I see there are lots of driver named with this format in kernel, such as dw_hdmi & dw_mmc >>> Please look at other dw cases. >>> For example, look at dw_pcie. >>> >>> drivers/pci/host/ >>> pcie-designware.c >>> pci-spear13xx.c >>> pci-exynos.c >>> >>> In this case, pci-spear13xx.c and pci-exynos.c do not use >>> "IP_name+SoC_name", even though these are dw IPs. >>> >>> Also, naming consistency is more important. >>> Now, Exynos DRM files are using "exynos_drm_" prefix. >>> >>> drivers/gpu/drm/exynos/ >>> exynos_drm_buf.c >>> exynos_drm_core.c >>> >>> >>> However, "analogix_dp-exynos.c" looks very inconsistent. >>> >>> If there is no strict naming rule, please use "exynos_dp.c" >>> or "exynos_drm_dp.c". >> Exynos DRM maintainers get to pick their filenames, so Yakir, please >> rename as Jingoo suggested. >> >> Even if you didn't the first thing that would go into the Exynos DRM >> driver tree after this is merged is a rename patch anyway. >> >> Thierry > >
[PATCH] amdgpu: fix missing deinit on vamgr_32
From: "monk.liu"
Signed-off-by: monk.liu
Signed-off-by: Alex Deucher
---
amdgpu/amdgpu_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index 75b12e2..e5a923e 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -132,6 +132,8 @@ static void
amdgpu_device_free_internal(amdgpu_device_handle dev)
{
amdgpu_vamgr_deinit(dev->vamgr);
free(dev->vamgr);
+ amdgpu_vamgr_deinit(dev->vamgr_32);
+ free(dev->vamgr_32);
util_hash_table_destroy(dev->bo_flink_names);
util_hash_table_destroy(dev->bo_handles);
pthread_mutex_destroy(&dev->bo_table_mutex);
--
1.8.3.1
[PATCH 1/1] signals: kill block_all_signals() and unblock_all_signals()
On Thu, Aug 27, 2015 at 06:25:29PM +0200, Oleg Nesterov wrote:
> It is hardly possible to enumerate all problems with block_all_signals()
> and unblock_all_signals(). Just for example,
>
> 1. block_all_signals(SIGSTOP/etc) simply can't help if the caller is
>multithreaded. Another thread can dequeue the signal and force the
>group stop.
>
> 2. Even is the caller is single-threaded, it will "stop" anyway. It
>will not sleep, but it will spin in kernel space until SIGCONT or
>SIGKILL.
>
> And a lot more. In short, this interface doesn't work at all, at least
> the last 10+ years.
>
> Signed-off-by: Oleg Nesterov
Yeah the only times I played around with the DRM_LOCK stuff was when old
drivers accidentally deadlocked - my impression is that the entire
DRM_LOCK thing was never really tested properly ;-) Hence I'm all for
purging where this leaks out of the drm subsystem.
Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_lock.c | 41 ---
> include/drm/drmP.h |1 -
> include/linux/sched.h |7 +-
> kernel/signal.c| 51
> +---
> 4 files changed, 2 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
> index f861361..4477b87 100644
> --- a/drivers/gpu/drm/drm_lock.c
> +++ b/drivers/gpu/drm/drm_lock.c
> @@ -38,8 +38,6 @@
> #include "drm_legacy.h"
> #include "drm_internal.h"
>
> -static int drm_notifier(void *priv);
> -
> static int drm_lock_take(struct drm_lock_data *lock_data, unsigned int
> context);
>
> /**
> @@ -115,14 +113,8 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
>* really probably not the correct answer but lets us debug xkb
>* xserver for now */
> if (!file_priv->is_master) {
> - sigemptyset(&dev->sigmask);
> - sigaddset(&dev->sigmask, SIGSTOP);
> - sigaddset(&dev->sigmask, SIGTSTP);
> - sigaddset(&dev->sigmask, SIGTTIN);
> - sigaddset(&dev->sigmask, SIGTTOU);
> dev->sigdata.context = lock->context;
> dev->sigdata.lock = master->lock.hw_lock;
> - block_all_signals(drm_notifier, dev, &dev->sigmask);
> }
>
> if (dev->driver->dma_quiescent && (lock->flags & _DRM_LOCK_QUIESCENT))
> @@ -163,7 +155,6 @@ int drm_legacy_unlock(struct drm_device *dev, void *data,
> struct drm_file *file_
> /* FIXME: Should really bail out here. */
> }
>
> - unblock_all_signals();
> return 0;
> }
>
> @@ -282,38 +273,6 @@ int drm_legacy_lock_free(struct drm_lock_data
> *lock_data, unsigned int context)
> }
>
> /**
> - * If we get here, it means that the process has called DRM_IOCTL_LOCK
> - * without calling DRM_IOCTL_UNLOCK.
> - *
> - * If the lock is not held, then let the signal proceed as usual. If the
> lock
> - * is held, then set the contended flag and keep the signal blocked.
> - *
> - * \param priv pointer to a drm_device structure.
> - * \return one if the signal should be delivered normally, or zero if the
> - * signal should be blocked.
> - */
> -static int drm_notifier(void *priv)
> -{
> - struct drm_device *dev = priv;
> - struct drm_hw_lock *lock = dev->sigdata.lock;
> - unsigned int old, new, prev;
> -
> - /* Allow signal delivery if lock isn't held */
> - if (!lock || !_DRM_LOCK_IS_HELD(lock->lock)
> - || _DRM_LOCKING_CONTEXT(lock->lock) != dev->sigdata.context)
> - return 1;
> -
> - /* Otherwise, set flag to force call to
> -drmUnlock */
> - do {
> - old = lock->lock;
> - new = old | _DRM_LOCK_CONT;
> - prev = cmpxchg(&lock->lock, old, new);
> - } while (prev != old);
> - return 0;
> -}
> -
> -/**
> * This function returns immediately and takes the hw lock
> * with the kernel context if it is free, otherwise it gets the highest
> priority when and if
> * it is eventually released.
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 62c4077..0859c35 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -815,7 +815,6 @@ struct drm_device {
>
> struct drm_sg_mem *sg; /**< Scatter gather memory */
> unsigned int num_crtcs; /**< Number of CRTCs on this
> device */
> - sigset_t sigmask;
>
> struct {
> int context;
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 26a2e61..f192cfe 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1489,9 +1489,7 @@ struct task_struct {
>
> unsigned long sas_ss_sp;
> size_t sas_ss_size;
> - int (*notifier)(void *priv);
> - void *notifier_data;
> - sigset_t *notifier_mask;
> +
> struct callback_head *task_works;
>
> struct audit_context *audit_context;
> @@ -2382,9 +2380,6 @@ static inline int dequeue_signal_lock(struct
> task_struct *t
[Intel-gfx] drm/atomic: Reject events for inactive crtc's.
Op 27-08-15 om 17:22 schreef Daniel Vetter:
> On Thu, Aug 27, 2015 at 03:36:09PM +0100, Daniel Stone wrote:
>> Hi,
>>
>> On 6 August 2015 at 13:49, Daniel Vetter wrote:
>>> On Thu, Aug 06, 2015 at 01:19:35PM +0200, Maarten Lankhorst wrote:
Op 06-08-15 om 11:47 schreef Daniel Stone:
> On 30 July 2015 at 08:03, Maarten Lankhorst
> wrote:
>> + if (!state->active && state->event) {
>> + DRM_DEBUG_ATOMIC("[CRTC:%d] requesting event and not
>> active\n",
>> +crtc->base.id);
>> + return -EINVAL;
>> + }
> Hmm, even if disabling? Maybe (!crtc->state->active && !state->active)
> && state->event.
How do you want to send a vblank event after disabling?
>>> The event would be when we stop scanning out, but yeah that's a bit a
>>> tricky one. I guess for now (until we have someone who needs this) we
>>> could just merge Maarten's patch as the easier thing to do right now?
>>> Maybe with a small code comment that this is intentional?
>> Exactly that. Surely this (when the CRTC actually goes dark) is
>> something we already know? Assuming you don't have atomic_disable /
>> instant-scanout-stop, and have to wait until the next vblank to kill
>> it, then how does this work?
>> - create FB
>> - assign FB to plane on CRTC
>> - unreference FB such that plane holds the last remaining reference
>> - set plane->fb == NULL + crtc->active = 0, i.e. SetCrtc(NULL, 0, 0,
>> 0, 0, ...)
>>
>> You can't immediately unpin/destroy the FB since you might still be
>> mid-scanout. So you already need to defer this, and that would be the
>> point at which you stop scanout. In a lot of hardware, this is just
>> another latched register which takes effect on the next vblank, for
>> which you still catch an IRQ - at which point you send the event. If
>> you actually have atomic_disable hardware that stops scanout
>> immediately, you can just send the event immediately.
>>
>> What am I missing?
> The userspace which actually wants this and the testcases which makes sure
> it works. Until we have that I'd really prefer to just close that
> undefined behaviour gap in the atomic api, similar to how we just merged a
> patch to disallow switching live planes.
>
> We can always extend this later on easily, there's lots of little features
> like this that people suggested for atomic.
> -Daniel
Or in this case it's really solved in the kernel. Old fb's don't get unpinned
until the update, so why should userspace care about any of it, assuming kernel
refcount is correct?
~Maarten
[Bug 91808] trine1 misrender r600g
https://bugs.freedesktop.org/show_bug.cgi?id=91808 --- Comment #3 from Martin Bednar --- >From what I gather, 66067 is about colors & luminosity. I reported this because of the line that goes vertically through the whole screen. On the screenshot it is located under "paused". The sources of the line are AFAICS the gears just above the spiky area. Specifying now just to make sure that we're talking about the same thing. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150831/844c9782/attachment.html>
[PATCH] drm/msm/mdp5: enable clocks in hw_init and set_irqmask
On Mon, Aug 31, 2015 at 1:15 AM, Archit Taneja
wrote:
>
>
> On 08/28/2015 11:48 PM, Rob Clark wrote:
>>
>> On Fri, Aug 28, 2015 at 3:56 AM, Archit Taneja
>> wrote:
>>>
>>>
>>>
>>> On 08/27/2015 10:36 AM, Archit Taneja wrote:
On 08/26/2015 08:42 PM, hali at codeaurora.org wrote:
>>
>>
>> 2015-08-26 9:55 GMT-04:00 :
>>>
>>>
>>> Hi Archit,
>>>
mdp5_hw_init and mdp5_set_irqmask configure registers but may not
have
clocks enabled.
Add mdp5_enable/disable calls in these funcs to ensure clocks are
enabled. We need this until we get proper runtime pm support.
Signed-off-by: Archit Taneja
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 10 --
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
index b1f73be..9fabfca 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c
@@ -24,9 +24,15 @@
void mdp5_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask,
uint32_t old_irqmask)
{
- mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_MDP_INTR_CLEAR(0),
+ struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms);
+
+ mdp5_enable(mdp5_kms);
+
+ mdp5_write(mdp5_kms, REG_MDP5_MDP_INTR_CLEAR(0),
irqmask ^ (irqmask & old_irqmask));
- mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_MDP_INTR_EN(0),
irqmask);
+ mdp5_write(mdp5_kms, REG_MDP5_MDP_INTR_EN(0), irqmask);
+
+ mdp5_disable(mdp5_kms);
}
>>>
>>>
>>>
>>> mdp5_set_irqmask() can be invoked in atomic context, clk_prepare() is
>>> not
>>> allowed in this function because it may cause process to sleep. We
>>> can
>>> enable the clocks in the caller at initialization.
Oh, oops. I missed that.
>>
>> iirc, it will be called with at least one spinlock held..
>>
>> We do already move the enable/disable_vblank() paths off to a worker
>> so that we can ensure things are enabled before we get into
>> update_irq().. the only other path to update_irq() should be when
>> driver code does mdp_irq_register/unregister().. so maybe we should
>> just require that the mdp4/mdp5 kms code only calls those when clk's
>> are already enabled (which should be mostly true already, I think)
>>
>> BR,
>> -R
>
>
>
> Yes, the only case that not been covered is mdp5_irq_postinstall(). We
> can
> enable clocks in this function. Actually, this is what we are doing in
> downstream test.
It works fine if I put it in postinstall. I'll update the patch and
resend.
>>>
>>>
>>>
>>> So, I hit an issue in both the approaches.
>>>
>>> When I try modeset, I get a watchdog reset once the app closes down.
>
>
> I meant modetest here*
>
>>>
>>> Looking at debug logs, it looks like the issue happens when the ioctl
>>> RMFB and drm_release race with each other.
>>
>>
>> hmm, this seems a bit strange.. since to do the RMFB ioctl the device
>> must still be open.. do we end up w/ the RMFB being an async commit
>> somehow? (Although in case of flip w/ gpu rendering still pending,
>> somewhere we probably want to block on previous async commit?)
>
>
> It isn't a race condition as I thought before. The RMFB ioctl isn't
> asynchronous either.
>
> The problem has to do with how msm_atomic_commit behaves when the
> state we want to commit involves disabling the crtc:
>
> In this case, we grab clocks once (in mdp5_prepare_commit), but
> disable them twice (first, via mdp5_crtc_disable() in disable_outputs(),
> and then in mdp5_complete_commit())
We need a better way in disable path to wait until scanout has
finished.. I'm not sure if there is a good way to do this. I seem to
recall seeing some code in the downstream android driver which would
spin on some (iirc) clk status bits, or something like that.
(Although presumably that involves disabling clk first.)
There is a slightly related issue that, since we don't know when
scanout completes, we end up unmapping scanout buffers too early and
getting some iommu faults (which so far have been mostly harmless, but
ugly)
If there was a non-racy way to wait for last vblank irq, I suppose we
could do that. Or maybe we should just msleep() the expected amount
of time given the refresh rate.
BR,
-R
> Whatever comes next will crash if it requires clocks. In this case,
> drm_release's call to mdp5_crtc_cancel_pending_flip() is the first
> thing that needs clocks, and it crashes there.
>
>>
>>> Within the the msm driver, thi
[Bug 91808] trine1 misrender r600g
https://bugs.freedesktop.org/show_bug.cgi?id=91808 --- Comment #2 from Alex Deucher --- Possibly a duplicate of bug 66067. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150831/e3bab210/attachment.html>
[Bug 91808] trine1 misrender r600g
https://bugs.freedesktop.org/show_bug.cgi?id=91808 Alex Deucher changed: What|Removed |Added Attachment #117987|text/plain |image/png mime type|| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20150831/b6527edb/attachment-0001.html>
[GIT PULL] exynos-drm-next
Hi Dave,
This is a second pull-request which adds last part of
atomic modeset/pageflip support, render node support,
clean-up, and fix-up.
Please, kindly let me know if there is any problem.
Thanks,
Inki Dae
The following changes since commit 92cffd56b21c825579f3b37bc7803e4c37073076:
drm/nouveau/dispnv04: fix build on powerpc (2015-08-28 20:33:58 +1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
exynos-drm-next
for you to fetch changes up to 50002d4c2176da6b2ed5a2529a2367c05f0fd73b:
drm/exynos: fix build warning to exynos_drm_gem.c (2015-08-31 01:12:36 +0900)
Gustavo Padovan (12):
drm/exynos: don't track enabled state at exynos_crtc
drm/exynos: fimd: unify call to exynos_drm_crtc_finish_pageflip()
drm/exynos: add prepare and cleanup phases for planes
drm/exynos: fimd: move window protect code to prepare/cleanup_plane
drm/exynos: check for pending fb before finish update
drm/exynos: add macro to get the address of START_S reg
drm/exynos: fimd: only finish update if START == START_S
drm/exynos: add atomic asynchronous commit
drm/exynos: wait all planes updates to finish
drm/exynos: remove wait queue for pending page flip
drm/exynos: Enable atomic modesetting feature
drm/exynos: remove legacy ->suspend()/resume()
Hyungwon Hwang (1):
drm/exynos: implement atomic_{begin/flush} of DECON
Inki Dae (1):
drm/exynos: fix build warning to exynos_drm_gem.c
Joonyoung Shim (1):
drm/exynos: add render node support
Marek Szyprowski (1):
drm/exynos: Properly report supported formats for each device
drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 54 +--
drivers/gpu/drm/exynos/exynos7_drm_decon.c| 55 ++-
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 69 -
drivers/gpu/drm/exynos/exynos_drm_crtc.h |4 +-
drivers/gpu/drm/exynos/exynos_drm_drv.c | 196 ++---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 24 ++-
drivers/gpu/drm/exynos/exynos_drm_fb.c| 35 -
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 92
drivers/gpu/drm/exynos/exynos_drm_gem.c |2 +-
drivers/gpu/drm/exynos/exynos_drm_plane.c | 13 +-
drivers/gpu/drm/exynos/exynos_drm_plane.h |1 +
drivers/gpu/drm/exynos/exynos_drm_vidi.c | 19 ++-
drivers/gpu/drm/exynos/exynos_mixer.c | 40 -
include/video/samsung_fimd.h |1 +
14 files changed, 452 insertions(+), 153 deletions(-)
[PATCH] drm/exynos: fix exynos_drm_gem_prime_import_sg_table() error handling
2015-08-27 17:31 GMT+09:00 Joonyoung Shim :
> If exynos_drm_gem_init() is failed, the result is ERR_PTR, so we should
> just return the result. If not, wrong porinter will be referenced from
> err label.
>
> Reported-by: Dan Carpenter
> Signed-off-by: Joonyoung Shim
> ---
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 3e4a64a..4842a31 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -569,10 +569,8 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device
> *dev,
> int ret;
>
> exynos_gem_obj = exynos_drm_gem_init(dev, attach->dmabuf->size);
> - if (IS_ERR(exynos_gem_obj)) {
> - ret = PTR_ERR(exynos_gem_obj);
> - goto err;
> - }
> + if (IS_ERR(exynos_gem_obj))
> + return exynos_gem_obj;
This patch incurs below build warning,
drivers/gpu/drm/exynos/exynos_drm_gem.c: In function
'exynos_drm_gem_prime_import_sg_table':
drivers/gpu/drm/exynos/exynos_drm_gem.c:670:3: warning: return from
incompatible pointer type [enabled by default]
We can simply return ERR_PTR(ret) and I just fixed it.
>
> exynos_gem_obj->dma_addr = sg_dma_address(sgt->sgl);
>
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/2] drm: bridge/dw_hdmi: add dw hdmi i2c bus adapter support
The change adds support of internal HDMI I2C master controller, this
subdevice is used by default, if "ddc-i2c-bus" DT property is omitted.
The main purpose of this functionality is to support reading EDID from
an HDMI monitor on boards, which don't have an I2C bus connected to
DDC pins.
The current implementation does not support "I2C Master Interface
Extended Read Mode" to read data addressed by non-zero segment
pointer, this means that if EDID has more than 1 extension blocks,
EDID reading operation won't succeed, in my practice all tested HDMI
monitors have at maximum one extension block.
Signed-off-by: Vladimir Zapolskiy
---
The change is based on today's v4.2, please let me know, if it should be
rebased.
The change has compilation dependency on I2CM_ADDRESS register name fix,
see http://lists.freedesktop.org/archives/dri-devel/2015-May/082980.html
>From http://lists.freedesktop.org/archives/dri-devel/2015-May/082981.html
v1 of the change was
Tested-by: Philipp Zabel
but I hesitate to add the tag here due to multiple updates in v2.
Changes from v2 to v3, thanks to Russell:
- moved register field value definitions to dw_hdmi.h
- made completions uninterruptible to avoid transfer retries if interrupted
- use one completion for both read and write transfers as in v1, operation_reg
is removed
- redundant i2c->stat = 0 is removed from dw_hdmi_i2c_read/write()
- struct i2c_algorithm dw_hdmi_algorithm is qualified as const
- dw_hdmi_i2c_adapter() does not modify struct dw_hdmi on error path
- dw_hdmi_i2c_irq() does not modify hdmi->i2c->stat, if interrupt is not for
I2CM
- spin lock is removed from dw_hdmi_i2c_irq()
- removed spin lock from dw_hdmi_i2c_xfer() around write to
HDMI_IH_MUTE_I2CM_STAT0 register
- split loop over message array in dw_hdmi_i2c_xfer() to validation and
transfer parts
- added a mutex to serialize I2C transfer requests, i2c->lock is completely
removed
- removed if(len) check from dw_hdmi_i2c_write(), hardware supports only len>0
transfers
- described extension blocks <= 1 limitation in the commit message
- a number of minor clean ups
Changes from v1 to v2:
- fixed a devm_kfree() signature
- split completions for read and write operations
drivers/gpu/drm/bridge/dw_hdmi.c | 262 ++-
drivers/gpu/drm/bridge/dw_hdmi.h | 19 +++
2 files changed, 275 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 816d104..d329e04 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -1,14 +1,15 @@
/*
+ * DesignWare High-Definition Multimedia Interface (HDMI) driver
+ *
+ * Copyright (C) 2013-2015 Mentor Graphics Inc.
* Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
+ * Copyright (C) 2010, Guennadi Liakhovetski
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
- * Designware High-Definition Multimedia Interface (HDMI) driver
- *
- * Copyright (C) 2010, Guennadi Liakhovetski
*/
#include
#include
@@ -102,6 +103,17 @@ struct hdmi_data_info {
struct hdmi_vmode video_mode;
};
+struct dw_hdmi_i2c {
+ struct i2c_adapter adap;
+
+ struct mutexlock;
+ struct completion cmp;
+ u8 stat;
+
+ u8 slave_reg;
+ boolis_regaddr;
+};
+
struct dw_hdmi {
struct drm_connector connector;
struct drm_encoder *encoder;
@@ -111,6 +123,7 @@ struct dw_hdmi {
struct device *dev;
struct clk *isfr_clk;
struct clk *iahb_clk;
+ struct dw_hdmi_i2c *i2c;
struct hdmi_data_info hdmi_data;
const struct dw_hdmi_plat_data *plat_data;
@@ -179,6 +192,200 @@ static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u8
data, unsigned int reg,
hdmi_modb(hdmi, data << shift, mask, reg);
}
+static void dw_hdmi_i2c_init(struct dw_hdmi *hdmi)
+{
+ /* Set Fast Mode speed */
+ hdmi_writeb(hdmi, 0x0b, HDMI_I2CM_DIV);
+
+ /* Software reset */
+ hdmi_writeb(hdmi, 0x00, HDMI_I2CM_SOFTRSTZ);
+
+ /* Set done, not acknowledged and arbitration interrupt polarities */
+ hdmi_writeb(hdmi, HDMI_I2CM_INT_DONE_POL, HDMI_I2CM_INT);
+ hdmi_writeb(hdmi, HDMI_I2CM_CTLINT_NAC_POL | HDMI_I2CM_CTLINT_ARB_POL,
+ HDMI_I2CM_CTLINT);
+
+ /* Clear DONE and ERROR interrupts */
+ hdmi_writeb(hdmi, HDMI_IH_I2CM_STAT0_ERROR | HDMI_IH_I2CM_STAT0_DONE,
+ HDMI_IH_I2CM_STAT0);
+
+ /* Mute DONE and ERROR interrupts */
+ hdmi_writeb(hdmi, HDMI_IH_I2CM_STAT0_ERROR | HDMI_IH_I2CM_STAT0_DONE,
+ HDMI_IH_MUTE_I2CM_STAT0);
+}
+
+static int dw_hdmi_i2c_read(struct dw_hdmi *hdmi,
+
