Re: [PATCH v3] drm/exynos: mixer: configure layers once in mixer_atomic_flush()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27.09.2016 13:22, Tobias Jakobi wrote:
> Hello Inki,
>
>
> Inki Dae wrote:
>> 2016년 09월 26일 20:36에 Tobias Jakobi 이(가) 쓴 글:
>>> Only manipulate the MXR_CFG and MXR_LAYER_CFG registers once
>>> in mixer_cfg_layer().
>>> Trigger this via atomic flush.
>>>
>>> Changes in v2:
>>> - issue mixer_cfg_layer() in mixer_disable()
>>> - rename fields as suggested by Andrzej
>>> - added docu to mixer context struct
>>> - simplify mixer_win_reset() as well
>>>
>>> Changes in v3:
>>> - simplify some conditions as suggested by Inki
>>> - add docu to mixer_cfg_layer()
>>> - fold switch statements into a single one
>>>
>>> Signed-off-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_mixer.c | 135 ++++++++++++++++++++++------------
>>>  drivers/gpu/drm/exynos/regs-mixer.h   |   2 +
>>>  2 files changed, 92 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> index 1e78d57..4f06f4d 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> @@ -93,12 +93,25 @@ static const uint32_t vp_formats[] = {
>>>  	DRM_FORMAT_NV21,
>>>  };
>>>  
>>> +/*
>>> + * Mixer context structure.
>>> + *
>>> + * @crtc: The HDMI CRTC attached to the mixer.
>>> + * @planes: Array of plane objects for each of the mixer windows.
>>> + * @active_windows: Cache of the mixer's hardware state.
>>> + *       Tracks which mixer windows are active/inactive.
>>> + * @pipe: The CRTC index.
>>> + * @flags: Bitfield build from the mixer_flag_bits enumerator.
>>> + * @mixer_resources: A struct containing registers, clocks, etc.
>>> + * @mxr_ver: The hardware revision/version of the mixer.
>>> + */
>>>  struct mixer_context {
>>>  	struct platform_device *pdev;
>>>  	struct device		*dev;
>>>  	struct drm_device	*drm_dev;
>>>  	struct exynos_drm_crtc	*crtc;
>>>  	struct exynos_drm_plane	planes[MIXER_WIN_NR];
>>> +	unsigned long		active_windows;
>>>  	int			pipe;
>>>  	unsigned long		flags;
>>>  
>>> @@ -418,37 +431,70 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height)
>>>  	mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_RGB_FMT_MASK);
>>>  }
>>>  
>>> -static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
>>> -			    unsigned int priority, bool enable)
>>> +/**
>>> + * mixer_cfg_layer - apply layer configuration to hardware
>>> + * @ctx: mixer context
>>> + *
>>> + * This configures the MXR_CFG and MXR_LAYER_CFG hardware registers
>>> + * using the 'active_windows' field of the the mixer content, and
>>> + * the pixel format of the framebuffers associated with the enabled
>>> + * windows.
>>> + *
>>> + * Has to be called under mixer lock.
>>> + */
>>> +static void mixer_cfg_layer(struct mixer_context *ctx)
>>>  {
>>>  	struct mixer_resources *res = &ctx->mixer_res;
>>> -	u32 val = enable ? ~0 : 0;
>>> -
>>> -	switch (win) {
>>> -	case 0:
>>> -		mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
>>> -		mixer_reg_writemask(res, MXR_LAYER_CFG,
>>> -				    MXR_LAYER_CFG_GRP0_VAL(priority),
>>> -				    MXR_LAYER_CFG_GRP0_MASK);
>>> -		break;
>>> -	case 1:
>>> -		mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP1_ENABLE);
>>> -		mixer_reg_writemask(res, MXR_LAYER_CFG,
>>> -				    MXR_LAYER_CFG_GRP1_VAL(priority),
>>> -				    MXR_LAYER_CFG_GRP1_MASK);
>>> +	unsigned int win;
>>>  
>>> -		break;
>>> -	case VP_DEFAULT_WIN:
>>> -		if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) {
>>> -			vp_reg_writemask(res, VP_ENABLE, val, VP_ENABLE_ON);
>>> -			mixer_reg_writemask(res, MXR_CFG, val,
>>> -				MXR_CFG_VP_ENABLE);
>>> -			mixer_reg_writemask(res, MXR_LAYER_CFG,
>>> -					    MXR_LAYER_CFG_VP_VAL(priority),
>>> -					    MXR_LAYER_CFG_VP_MASK);
>>> +	struct exynos_drm_plane_state *state;
>>> +	struct drm_framebuffer *fb;
>>> +	unsigned int priority;
>>> +	u32 mxr_cfg = 0, mxr_layer_cfg = 0, vp_enable = 0;
>>> +	bool enable;
>>> +
>>> +	for (win = 0; win < MIXER_WIN_NR; ++win) {
>>> +		state = to_exynos_plane_state(ctx->planes[win].base.state);
>>> +		fb = state->fb;
>>> +
>>> +		priority = state->base.normalized_zpos + 1;
>>> +		enable = test_bit(win, &ctx->active_windows);
>>> +
>>> +		if (!enable)
>>> +			continue;
>>> +
>>> +		BUG_ON(!fb);
>>> +
>>> +		/*
>>> +		 * TODO: Don't enable alpha blending for the bottom window.
>>> +		 */
>>> +		switch (win) {
>>> +		case 0:
>>> +			mxr_cfg |=  MXR_CFG_GRP0_ENABLE;
>>> +			mxr_layer_cfg |= MXR_LAYER_CFG_GRP0_VAL(priority);
>>> +			mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->pixel_format));
>>> +			break;
>>> +
>>> +		case 1:
>>> +			mxr_cfg |=  MXR_CFG_GRP1_ENABLE;
>>> +			mxr_layer_cfg |= MXR_LAYER_CFG_GRP1_VAL(priority);
>>> +			mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->pixel_format));
>>> +			break;
>>> +
>>> +		case VP_DEFAULT_WIN:
>>> +			vp_enable = VP_ENABLE_ON;
>>> +			mxr_cfg |=  MXR_CFG_VP_ENABLE;
>>> +			mxr_layer_cfg |= MXR_LAYER_CFG_VP_VAL(priority);
>>> +			mixer_cfg_vp_blend(ctx);
>>> +			break;
>>>  		}
>>> -		break;
>>>  	}
>>> +
>>> +	if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags))
>>> +		vp_reg_writemask(res, VP_ENABLE, vp_enable, VP_ENABLE_ON);
>>> +
>>> +	mixer_reg_writemask(res, MXR_CFG, mxr_cfg, MXR_CFG_ENABLE_MASK);
>>> +	mixer_reg_writemask(res, MXR_LAYER_CFG, mxr_layer_cfg, MXR_LAYER_CFG_MASK);
>>>  }
>>>  
>>>  static void mixer_run(struct mixer_context *ctx)
>>> @@ -478,7 +524,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
>>>  	struct drm_display_mode *mode = &state->base.crtc->state->adjusted_mode;
>>>  	struct mixer_resources *res = &ctx->mixer_res;
>>>  	struct drm_framebuffer *fb = state->fb;
>>> -	unsigned int priority = state->base.normalized_zpos + 1;
>>>  	unsigned long flags;
>>>  	dma_addr_t luma_addr[2], chroma_addr[2];
>>>  	bool tiled_mode = false;
>>> @@ -563,8 +608,6 @@ static void vp_video_buffer(struct mixer_context *ctx,
>>>  
>>>  	mixer_cfg_scan(ctx, mode->vdisplay);
>>>  	mixer_cfg_rgb_fmt(ctx, mode->vdisplay);
>>> -	mixer_cfg_layer(ctx, plane->index, priority, true);
>>> -	mixer_cfg_vp_blend(ctx);
>> You removed the function call to set MXR_CFG and MXR_LAYER_CFG registers so what if lagacy set plane ioctl is called by user user space? Where these register are updated?
> yes, you're right. I completly forgot about the legacy codepaths. Sorry
> about that!

Are you talking about DRM_IOCTL_MODE_SETPLANE?
It does not seem to be legacy, or to be more precise it calls
.update_plane and .disable_plane
callbacks which in exynos are set as follow:
    .update_plane    = drm_atomic_helper_update_plane,
    .disable_plane    = drm_atomic_helper_disable_plane,

So DRM_IOCTL_MODE_SETPLANE calls atomic codepath anyway.

Regards
Andrzej

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux