dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Joonyoung Shim <jy0922.shim@samsung.com>
To: InKi Dae <inki.dae@samsung.com>
Cc: kyungmin.park@samsung.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 11/13] drm/exynos: changed context name of hdmi and mixer
Date: Mon, 20 Aug 2012 15:36:29 +0900	[thread overview]
Message-ID: <5031DAED.7040606@samsung.com> (raw)
In-Reply-To: <CAAQKjZO3mLuq-4BXEzCGC2tufKq5_B8LhDKkFnPaEWYHpkyY4g@mail.gmail.com>

On 08/20/2012 03:17 PM, InKi Dae wrote:
> 2012/8/20 Joonyoung Shim <jy0922.shim@samsung.com>:
>> On 08/20/2012 11:29 AM, InKi Dae wrote:
>>> 2012/8/20 Joonyoung Shim <jy0922.shim@samsung.com>:
>>>> On 08/17/2012 06:50 PM, Inki Dae wrote:
>>>>> this patch changes ctx variable name in exynos_drm_hdmi_context
>>>>> structure to client because the use of ctx variable makes it confused.
>>>>
>>>> I don't prefer "client" name. This is not client and server relationship.
>>>>
>>> Okay, give me your opinion. which one do you prefer?
>>
>> Just "data".
>>
> It's not clear. "data" is so comprehensive just use "child_ctx". we
> already use "parent_ctx" as the context of exynos_drm_hdmi module.

Actually, i prefer "data" term than "ctx" term. Anyway i want to focus
why this problem is occurred because i think that codes are some ugly. I
think we first need to consider to solve this situation using code
redesign, structure change and etc.

Thanks.

>
>> Thanks.
>>
>>
>>>>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>> ---
>>>>>     drivers/gpu/drm/exynos/exynos_drm_hdmi.c |   38
>>>>> +++++++++++++++---------------
>>>>>     drivers/gpu/drm/exynos/exynos_drm_hdmi.h |    4 +-
>>>>>     drivers/gpu/drm/exynos/exynos_hdmi.c     |   12 ++++----
>>>>>     drivers/gpu/drm/exynos/exynos_mixer.c    |    8 +++---
>>>>>     4 files changed, 31 insertions(+), 31 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
>>>>> b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
>>>>> index 3fdf0b6..bced38e 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
>>>>> @@ -64,7 +64,7 @@ static bool drm_hdmi_is_connected(struct device *dev)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->is_connected)
>>>>> -               return hdmi_ops->is_connected(ctx->hdmi_ctx->ctx);
>>>>> +               return hdmi_ops->is_connected(ctx->hdmi_ctx->client);
>>>>>           return false;
>>>>>     }
>>>>> @@ -77,8 +77,8 @@ static int drm_hdmi_get_edid(struct device *dev,
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->get_edid)
>>>>> -               return hdmi_ops->get_edid(ctx->hdmi_ctx->ctx, connector,
>>>>> edid,
>>>>> -                                         len);
>>>>> +               return hdmi_ops->get_edid(ctx->hdmi_ctx->client,
>>>>> connector,
>>>>> +                                               edid, len);
>>>>>           return 0;
>>>>>     }
>>>>> @@ -90,7 +90,7 @@ static int drm_hdmi_check_timing(struct device *dev,
>>>>> void *timing)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->check_timing)
>>>>> -               return hdmi_ops->check_timing(ctx->hdmi_ctx->ctx,
>>>>> timing);
>>>>> +               return hdmi_ops->check_timing(ctx->hdmi_ctx->client,
>>>>> timing);
>>>>>           return 0;
>>>>>     }
>>>>> @@ -102,7 +102,7 @@ static int drm_hdmi_power_on(struct device *dev, int
>>>>> mode)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->power_on)
>>>>> -               return hdmi_ops->power_on(ctx->hdmi_ctx->ctx, mode);
>>>>> +               return hdmi_ops->power_on(ctx->hdmi_ctx->client, mode);
>>>>>           return 0;
>>>>>     }
>>>>> @@ -124,7 +124,7 @@ static int drm_hdmi_enable_vblank(struct device
>>>>> *subdrv_dev)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (mixer_ops && mixer_ops->enable_vblank)
>>>>> -               return mixer_ops->enable_vblank(ctx->mixer_ctx->ctx,
>>>>> +               return mixer_ops->enable_vblank(ctx->mixer_ctx->client,
>>>>>                                                   manager->pipe);
>>>>>           return 0;
>>>>> @@ -137,12 +137,12 @@ static void drm_hdmi_disable_vblank(struct device
>>>>> *subdrv_dev)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (mixer_ops && mixer_ops->disable_vblank)
>>>>> -               return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx);
>>>>> +               return
>>>>> mixer_ops->disable_vblank(ctx->mixer_ctx->client);
>>>>>     }
>>>>>       static void drm_hdmi_mode_fixup(struct device *subdrv_dev,
>>>>>                                   struct drm_connector *connector,
>>>>> -                               const struct drm_display_mode *mode,
>>>>> +                               struct drm_display_mode *mode,
>>>>>                                   struct drm_display_mode *adjusted_mode)
>>>>>     {
>>>>>           struct drm_hdmi_context *ctx = to_context(subdrv_dev);
>>>>> @@ -150,7 +150,7 @@ static void drm_hdmi_mode_fixup(struct device
>>>>> *subdrv_dev,
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->mode_fixup)
>>>>> -               hdmi_ops->mode_fixup(ctx->hdmi_ctx->ctx, connector,
>>>>> mode,
>>>>> +               hdmi_ops->mode_fixup(ctx->hdmi_ctx->client, connector,
>>>>> mode,
>>>>>                                        adjusted_mode);
>>>>>     }
>>>>>     @@ -161,7 +161,7 @@ static void drm_hdmi_mode_set(struct device
>>>>> *subdrv_dev, void *mode)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->mode_set)
>>>>> -               hdmi_ops->mode_set(ctx->hdmi_ctx->ctx, mode);
>>>>> +               hdmi_ops->mode_set(ctx->hdmi_ctx->client, mode);
>>>>>     }
>>>>>       static void drm_hdmi_get_max_resol(struct device *subdrv_dev,
>>>>> @@ -172,7 +172,7 @@ static void drm_hdmi_get_max_resol(struct device
>>>>> *subdrv_dev,
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->get_max_resol)
>>>>> -               hdmi_ops->get_max_resol(ctx->hdmi_ctx->ctx, width,
>>>>> height);
>>>>> +               hdmi_ops->get_max_resol(ctx->hdmi_ctx->client, width,
>>>>> height);
>>>>>     }
>>>>>       static void drm_hdmi_commit(struct device *subdrv_dev)
>>>>> @@ -182,7 +182,7 @@ static void drm_hdmi_commit(struct device
>>>>> *subdrv_dev)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (hdmi_ops && hdmi_ops->commit)
>>>>> -               hdmi_ops->commit(ctx->hdmi_ctx->ctx);
>>>>> +               hdmi_ops->commit(ctx->hdmi_ctx->client);
>>>>>     }
>>>>>       static void drm_hdmi_dpms(struct device *subdrv_dev, int mode)
>>>>> @@ -192,10 +192,10 @@ static void drm_hdmi_dpms(struct device
>>>>> *subdrv_dev,
>>>>> int mode)
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (mixer_ops && mixer_ops->dpms)
>>>>> -               mixer_ops->dpms(ctx->mixer_ctx->ctx, mode);
>>>>> +               mixer_ops->dpms(ctx->mixer_ctx->client, mode);
>>>>>           if (hdmi_ops && hdmi_ops->dpms)
>>>>> -               hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode);
>>>>> +               hdmi_ops->dpms(ctx->hdmi_ctx->client, mode);
>>>>>     }
>>>>>       static void drm_hdmi_apply(struct device *subdrv_dev)
>>>>> @@ -209,11 +209,11 @@ static void drm_hdmi_apply(struct device
>>>>> *subdrv_dev)
>>>>>                   if (!ctx->enabled[i])
>>>>>                           continue;
>>>>>                   if (mixer_ops && mixer_ops->win_commit)
>>>>> -                       mixer_ops->win_commit(ctx->mixer_ctx->ctx, i);
>>>>> +                       mixer_ops->win_commit(ctx->mixer_ctx->client,
>>>>> i);
>>>>>           }
>>>>>           if (hdmi_ops && hdmi_ops->commit)
>>>>> -               hdmi_ops->commit(ctx->hdmi_ctx->ctx);
>>>>> +               hdmi_ops->commit(ctx->hdmi_ctx->client);
>>>>>     }
>>>>>       static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {
>>>>> @@ -235,7 +235,7 @@ static void drm_mixer_mode_set(struct device
>>>>> *subdrv_dev,
>>>>>           DRM_DEBUG_KMS("%s\n", __FILE__);
>>>>>           if (mixer_ops && mixer_ops->win_mode_set)
>>>>> -               mixer_ops->win_mode_set(ctx->mixer_ctx->ctx, overlay);
>>>>> +               mixer_ops->win_mode_set(ctx->mixer_ctx->client,
>>>>> overlay);
>>>>>     }
>>>>>       static void drm_mixer_commit(struct device *subdrv_dev, int zpos)
>>>>> @@ -251,7 +251,7 @@ static void drm_mixer_commit(struct device
>>>>> *subdrv_dev, int zpos)
>>>>>           }
>>>>>           if (mixer_ops && mixer_ops->win_commit)
>>>>> -               mixer_ops->win_commit(ctx->mixer_ctx->ctx, win);
>>>>> +               mixer_ops->win_commit(ctx->mixer_ctx->client, win);
>>>>>           ctx->enabled[win] = true;
>>>>>     }
>>>>> @@ -269,7 +269,7 @@ static void drm_mixer_disable(struct device
>>>>> *subdrv_dev, int zpos)
>>>>>           }
>>>>>           if (mixer_ops && mixer_ops->win_disable)
>>>>> -               mixer_ops->win_disable(ctx->mixer_ctx->ctx, win);
>>>>> +               mixer_ops->win_disable(ctx->mixer_ctx->client, win);
>>>>>           ctx->enabled[win] = false;
>>>>>     }
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
>>>>> b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
>>>>> index a91c420..237434d 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
>>>>> @@ -33,12 +33,12 @@
>>>>>      * exynos hdmi common context structure.
>>>>>      *
>>>>>      * @drm_dev: pointer to drm_device.
>>>>> - * @ctx: pointer to the context of specific device driver.
>>>>> + * @client: pointer to the context of specific device driver.
>>>>>      *    this context should be hdmi_context or mixer_context.
>>>>>      */
>>>>>     struct exynos_drm_hdmi_context {
>>>>>           struct drm_device       *drm_dev;
>>>>> -       void                    *ctx;
>>>>> +       void                    *client;
>>>>>     };
>>>>>       struct exynos_hdmi_ops {
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>>> index bb504cb..7538489 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>>> @@ -2108,7 +2108,7 @@ static struct exynos_hdmi_ops hdmi_ops = {
>>>>>     static irqreturn_t hdmi_external_irq_thread(int irq, void *arg)
>>>>>     {
>>>>>           struct exynos_drm_hdmi_context *ctx = arg;
>>>>> -       struct hdmi_context *hdata = ctx->ctx;
>>>>> +       struct hdmi_context *hdata = ctx->client;
>>>>>           if (!hdata->get_hpd)
>>>>>                   goto out;
>>>>> @@ -2127,7 +2127,7 @@ out:
>>>>>     static irqreturn_t hdmi_internal_irq_thread(int irq, void *arg)
>>>>>     {
>>>>>           struct exynos_drm_hdmi_context *ctx = arg;
>>>>> -       struct hdmi_context *hdata = ctx->ctx;
>>>>> +       struct hdmi_context *hdata = ctx->client;
>>>>>           u32 intc_flag;
>>>>>           intc_flag = hdmi_reg_read(hdata, HDMI_INTC_FLAG);
>>>>> @@ -2295,7 +2295,7 @@ static int __devinit hdmi_probe(struct
>>>>> platform_device *pdev)
>>>>>           mutex_init(&hdata->hdmi_mutex);
>>>>>     -     drm_hdmi_ctx->ctx = (void *)hdata;
>>>>> +       drm_hdmi_ctx->client = (void *)hdata;
>>>>>           hdata->parent_ctx = (void *)drm_hdmi_ctx;
>>>>>           platform_set_drvdata(pdev, drm_hdmi_ctx);
>>>>> @@ -2395,7 +2395,7 @@ static int __devexit hdmi_remove(struct
>>>>> platform_device *pdev)
>>>>>     {
>>>>>           struct device *dev = &pdev->dev;
>>>>>           struct exynos_drm_hdmi_context *ctx =
>>>>> platform_get_drvdata(pdev);
>>>>> -       struct hdmi_context *hdata = ctx->ctx;
>>>>> +       struct hdmi_context *hdata = ctx->client;
>>>>>           DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__);
>>>>>     @@ -2417,7 +2417,7 @@ static int __devexit hdmi_remove(struct
>>>>> platform_device *pdev)
>>>>>     static int hdmi_suspend(struct device *dev)
>>>>>     {
>>>>>           struct exynos_drm_hdmi_context *ctx = get_hdmi_context(dev);
>>>>> -       struct hdmi_context *hdata = ctx->ctx;
>>>>> +       struct hdmi_context *hdata = ctx->client;
>>>>>           disable_irq(hdata->internal_irq);
>>>>>           disable_irq(hdata->external_irq);
>>>>> @@ -2434,7 +2434,7 @@ static int hdmi_suspend(struct device *dev)
>>>>>     static int hdmi_resume(struct device *dev)
>>>>>     {
>>>>>           struct exynos_drm_hdmi_context *ctx = get_hdmi_context(dev);
>>>>> -       struct hdmi_context *hdata = ctx->ctx;
>>>>> +       struct hdmi_context *hdata = ctx->client;
>>>>>           enable_irq(hdata->external_irq);
>>>>>           enable_irq(hdata->internal_irq);
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>>> b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>>> index 30fcc12..d3ff595 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>>>> @@ -864,7 +864,7 @@ static void mixer_finish_pageflip(struct drm_device
>>>>> *drm_dev, int crtc)
>>>>>     static irqreturn_t mixer_irq_handler(int irq, void *arg)
>>>>>     {
>>>>>           struct exynos_drm_hdmi_context *drm_hdmi_ctx = arg;
>>>>> -       struct mixer_context *ctx = drm_hdmi_ctx->ctx;
>>>>> +       struct mixer_context *ctx = drm_hdmi_ctx->client;
>>>>>           struct mixer_resources *res = &ctx->mixer_res;
>>>>>           u32 val, base, shadow;
>>>>>     @@ -909,7 +909,7 @@ out:
>>>>>     static int __devinit mixer_resources_init(struct
>>>>> exynos_drm_hdmi_context
>>>>> *ctx,
>>>>>                                    struct platform_device *pdev)
>>>>>     {
>>>>> -       struct mixer_context *mixer_ctx = ctx->ctx;
>>>>> +       struct mixer_context *mixer_ctx = ctx->client;
>>>>>           struct device *dev = &pdev->dev;
>>>>>           struct mixer_resources *mixer_res = &mixer_ctx->mixer_res;
>>>>>           struct resource *res;
>>>>> @@ -1035,7 +1035,7 @@ static int __devinit mixer_probe(struct
>>>>> platform_device *pdev)
>>>>>           mutex_init(&ctx->mixer_mutex);
>>>>>           ctx->dev = &pdev->dev;
>>>>> -       drm_hdmi_ctx->ctx = (void *)ctx;
>>>>> +       drm_hdmi_ctx->client = (void *)ctx;
>>>>>           platform_set_drvdata(pdev, drm_hdmi_ctx);
>>>>>     @@ -1070,7 +1070,7 @@ static int mixer_remove(struct platform_device
>>>>> *pdev)
>>>>>     static int mixer_suspend(struct device *dev)
>>>>>     {
>>>>>           struct exynos_drm_hdmi_context *drm_hdmi_ctx =
>>>>> get_mixer_context(dev);
>>>>> -       struct mixer_context *ctx = drm_hdmi_ctx->ctx;
>>>>> +       struct mixer_context *ctx = drm_hdmi_ctx->client;
>>>>>           mixer_poweroff(ctx);
>>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2012-08-20  6:36 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-17  9:50 [PATCH 00/13] updated exynos-drm-fixes Inki Dae
2012-08-17  9:50 ` [PATCH 01/13] drm/exynos: added device object to subdrv's remove callback as argument Inki Dae
2012-08-17  9:50 ` [PATCH 02/13] drm/exynos: separated subdrv->probe call and encoder/connector creation Inki Dae
2012-08-20  1:11   ` Joonyoung Shim
2012-08-20  1:52     ` InKi Dae
2012-08-20  1:59       ` Joonyoung Shim
2012-08-20  4:31         ` InKi Dae
2012-08-20  4:42           ` Joonyoung Shim
2012-08-20  5:43             ` InKi Dae
2012-08-17  9:50 ` [PATCH 03/13] drm/exynos: fixed page align bug Inki Dae
2012-08-17  9:50 ` [PATCH 04/13] drm/exynos: use empty function instead of drm_helper_connector_dpms Inki Dae
2012-08-20  1:12   ` Joonyoung Shim
2012-08-20  2:50     ` InKi Dae
2012-08-17  9:50 ` [PATCH 05/13] drm/exynos: removed exynos_drm_encoder_dpms call Inki Dae
2012-08-20  1:14   ` Joonyoung Shim
2012-08-20  2:00     ` InKi Dae
2012-08-17  9:50 ` [PATCH 06/13] drm/exynos: separeated fimd_power_on into some parts Inki Dae
2012-08-17  9:50 ` [PATCH 07/13] drm/exynos: control display power at connector module Inki Dae
2012-08-17  9:50 ` [PATCH 08/13] drm/exynos: make sure that hardware overlay for fimd is disabled Inki Dae
2012-08-20  2:09   ` Joonyoung Shim
2012-08-17  9:50 ` [PATCH 09/13] drm/exynos: check NV12M format specific to Exynos properly Inki Dae
2012-08-20  1:17   ` Joonyoung Shim
2012-08-20  2:23     ` InKi Dae
2012-08-20  4:52       ` Joonyoung Shim
2012-08-20  5:11         ` InKi Dae
2012-08-20  5:15           ` InKi Dae
2012-08-20  5:22             ` Joonyoung Shim
2012-08-20  5:45               ` InKi Dae
2012-08-17  9:50 ` [PATCH 10/13] drm/exynos: update crtc to plane safely Inki Dae
2012-08-20  1:25   ` Joonyoung Shim
2012-08-17  9:50 ` [PATCH 11/13] drm/exynos: changed context name of hdmi and mixer Inki Dae
2012-08-20  1:27   ` Joonyoung Shim
2012-08-20  2:29     ` InKi Dae
2012-08-20  4:55       ` Joonyoung Shim
2012-08-20  6:17         ` InKi Dae
2012-08-20  6:36           ` Joonyoung Shim [this message]
2012-08-20  6:51             ` InKi Dae
2012-08-20  7:30               ` InKi Dae
2012-08-17  9:50 ` [PATCH 12/13] drm/exynos: fixed build warning Inki Dae
2012-08-17  9:50 ` [PATCH 13/13] drm/exynos: make sure that hardware overlay for hdmi is disabled Inki Dae

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5031DAED.7040606@samsung.com \
    --to=jy0922.shim@samsung.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=inki.dae@samsung.com \
    --cc=kyungmin.park@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox