[PATCH 02/10] drm/exynos: added mode_fixup feature and code clean.

[InKi Dae]

Hi Dave.

2012년 3월 15일 오후 8:21, Inki Dae <inki.dae at samsung.com>님의 말:
>> -----Original Message-----
>> From: Dave Airlie [mailto:airlied at gmail.com]
>> Sent: Thursday, March 15, 2012 7:40 PM
>> To: Inki Dae
>> Cc: airlied at linux.ie; dri-devel at lists.freedesktop.org;
>> kyungmin.park at samsung.com; sw0312.kim at samsung.com
>> Subject: Re: [PATCH 02/10] drm/exynos: added mode_fixup feature and code
>> clean.
>>
>> >  };
>> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> > index de81883..2d9a0e6 100644
>> > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> > @@ -249,7 +249,11 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc,
>> struct drm_display_mode *mode,
>> >  {
>> >        DRM_DEBUG_KMS("%s\n", __FILE__);
>> >
>> > -       mode = adjusted_mode;
>> > +       /*
>> > +        * copy the mode data adjusted by mode_fixup() into crtc->mode
>> > +        * so that hardware can be seet to proper mode.
>> > +        */
>> > +       memcpy(&crtc->mode, adjusted_mode, sizeof(*adjusted_mode));
>>
>> This just seems wrong on lots of levels, if you get an error how are
>> you going to fallback?
>>
>> You should be passing the modes into the crtc_update function and
>> doing things like other drivers here.
>>
>> Dave.
>

I have looked into the part you pointed out but I think it's ok.
exynos_drm_crtc_update just sets mode values to overlay structure and
overlay structure is common data structure that all hardware can use
and exynos crtc has no hardware defendency so we could use it commoly
for all hardware such as FIMD, HDMI and Virtual Display. so with
exynos_drm_crtc_update call, any hardward isn't updated. if
exynos_drm_crtc_update is fail then adjusted_mode would be released
and back to saved_mode. only if crtc_func->mode_set is true then
hardware would be updated. please let me know if there is my missing
point.

Thanks,
Inki Dae



> Got it. I will check return value from crtc_update and if it gets an error
> then I will handle it properly.
>
> Thanks,
> Inki Dae
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel