[PATCH] drm/i915/gvt: Refine new_mmio_info to override the added MMIO_reg
Zhao, Yakui
yakui.zhao at intel.com
Thu May 31 03:00:00 UTC 2018
On 2018年06月01日 10:46, Colin Xu wrote:
> On 05/31/2018 10:25 AM, Zhao, Yakui wrote:
>
>> Hi,
>> Any comment about this patch?
>> After the overriding is supported, it can reduce the effect on
>> the other platform when adding one
>> mmio handle for one specific platform.
>>
>> Thanks
>> Yakui
>
> trade-off against the convenience:
> - Unnecessary delete and re-add.
> - For a specific mmio, it's unclear how it's handled until go through
> all init functions, which
> transfers the effort to debugging stage, especially for issue with low
> fail rate.
>
Thanks for your comments.
Considering the below scenario:
One platform needs to specify one new mmio_handler/attribute for one
mmio_reg while this mmio_reg exists one multi platforms. (Also exist on
the generic platform).
Based on the current implementation, we need to remove this mmio_reg
from the generic_mmio_info and then add it for each platform. (Of course
it is also ok to refine the callback mmio_handler and add the
platform-check). In such case it is not convenient to handle it.
If we need to handle it for multiple mmio_regs, it will become one
burden especially when the code needs to be rebased frequently.
If the overriding is supported, this will be quite simple. We only need
to care how to handle the required mmio_reg for one platform and there
is no side effect on the other platforms.
> BR
> Colin
>
>>> -----Original Message-----
>>> From: Zhao, Yakui
>>> Sent: Thursday, May 17, 2018 11:58 AM
>>> To: intel-gvt-dev at lists.freedesktop.org
>>> Cc: Zhao, Yakui <yakui.zhao at intel.com>
>>> Subject: [PATCH] drm/i915/gvt: Refine new_mmio_info to override the
>>> added
>>> MMIO_reg
>>>
>>> GVT-g uses new_mmio_info function to track the required the mmio_reg. If
>>> the required mmio_reg is already added, error is returned. If one
>>> platform
>>> hopes to add new specific MMIO read/write callback for one mmio_reg,
>>> the
>>> change needs to be applied on all the platforms.
>>>
>>> So the new_mmio_info is implemented as override. If it is already
>>> added, it
>>> will be replaced by the new attributes(mask, read/write callback and
>>> so on).
>>>
>>> Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gvt/handlers.c | 25 +++++++++++--------------
>>> 1 file changed, 11 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
>>> b/drivers/gpu/drm/i915/gvt/handlers.c
>>> index 653ba2b..a7bf6d5 100644
>>> --- a/drivers/gpu/drm/i915/gvt/handlers.c
>>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>>> @@ -107,22 +107,19 @@ static int new_mmio_info(struct intel_gvt *gvt,
>>> end = offset + size;
>>>
>>> for (i = start; i < end; i += 4) {
>>> - info = kzalloc(sizeof(*info), GFP_KERNEL);
>>> - if (!info)
>>> - return -ENOMEM;
>>> -
>>> - info->offset = i;
>>> - p = find_mmio_info(gvt, info->offset);
>>> + p = find_mmio_info(gvt, i);
>>> if (p) {
>>> - WARN(1, "dup mmio definition offset %x\n",
>>> - info->offset);
>>> - kfree(info);
>>> + /* It will be removed from hash_list if found */
>>> + hash_del(&p->node);
>>> + info = p;
>>> + gvt_dbg_mmio("VGPU MMIO_reg %x is added.
>>> Override\n",
>>> + p->offset);
>>> + } else {
>>> + info = kzalloc(sizeof(*info), GFP_KERNEL);
>>> + if (!info)
>>> + return -ENOMEM;
>>>
>>> - /* We return -EEXIST here to make GVT-g load fail.
>>> - * So duplicated MMIO can be found as soon as
>>> - * possible.
>>> - */
>>> - return -EEXIST;
>>> + info->offset = i;
>>> }
>>>
>>> info->ro_mask = ro_mask;
>>> --
>>> 2.7.4
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
>
More information about the intel-gvt-dev
mailing list